From 203d67c45f3ed621b178ed2eb937f0b0deba5f57 Mon Sep 17 00:00:00 2001 From: jubnl Date: Mon, 27 Jul 2026 19:47:01 +0200 Subject: [PATCH] chore: move reservations backend to proper DI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Folds the 626-line services/reservationService.ts into the DI-native ReservationsService — byte-identical SQL, statuses, bodies, error strings and broadcasts; the legacy file is deleted. - reservations.mcp.ts: the 5 legacy tools (create/update/delete_reservation, reorder_reservations, link_hotel_accommodation) + the trip-reservations resource move to the decorator registry (identical names, schemas, annotations, error strings, broadcasts); the canWrite/canRead registration gates become declarative access markers. Legacy registrar and resource entries removed. - reservations.bridge.ts: 9 legacy-named exports + 3 type re-exports for the outside-container consumers (tripService, airtrail import/sync, the still-legacy transit + transports registrars, transitItineraryService's type-only import). - notifyBookingChange folds to the legacy (tripId, actorId) signature; the controller passes user.id and the plugin-host factory drops its last plain-function reservation import for a private method over the injected service (constructor unchanged). - TripsService and BookingImportService inject ReservationsService instead of importing legacy functions. - Tests: reservations.service.test.ts rewritten as a real-SQL suite (RESV-SVC-001..030, RES-TRAV-001..005 moved verbatim, RESV-BRIDGE-001..009 pinning the bridge); both e2e suites converted to the DI-native pattern; path mocks repointed to the bridge or constructor stubs; tools-reservations kept and extended with the moved resource cases. Quirks preserved on purpose (parity first — see the follow-up fix commit): the raw-accommodation_id metadata-sync gate, the un-transactioned multi-statement writes, the truthy updatePositions dayId check, the empty-string COALESCE keeps, and the TEXT accommodation_id normalization. --- server/src/mcp/resources.ts | 15 +- server/src/mcp/tools.ts | 4 +- server/src/mcp/tools/reservations.ts | 227 ------ server/src/mcp/tools/transit.ts | 2 +- server/src/mcp/tools/transports.ts | 2 +- server/src/nest/README.md | 19 +- .../booking-import/booking-import.module.ts | 3 +- .../booking-import/booking-import.service.ts | 5 +- server/src/nest/plugins/DI-MIGRATION.md | 5 +- .../plugins/host/plugin-host-deps.factory.ts | 19 +- .../nest/reservations/reservations.bridge.ts | 62 ++ .../reservations/reservations.controller.ts | 6 +- .../src/nest/reservations/reservations.mcp.ts | 285 +++++++ .../nest/reservations/reservations.module.ts | 8 +- .../nest/reservations/reservations.service.ts | 709 ++++++++++++++++- server/src/nest/trips/trips.module.ts | 3 +- server/src/nest/trips/trips.service.ts | 5 +- .../src/services/airtrail/airtrailImport.ts | 2 +- server/src/services/airtrail/airtrailSync.ts | 2 +- server/src/services/reservationService.ts | 626 --------------- .../src/services/transitItineraryService.ts | 2 +- server/src/services/tripService.ts | 2 +- server/tests/e2e/trips.e2e.test.ts | 27 +- server/tests/helpers/mcp-test-controllers.ts | 3 + server/tests/unit/mcp/resources.test.ts | 28 +- .../tests/unit/mcp/tools-reservations.test.ts | 34 +- server/tests/unit/mcp/tools-transit.test.ts | 4 +- .../booking-import.service.test.ts | 4 +- .../unit/nest/reservations.controller.test.ts | 4 +- .../unit/nest/reservations.service.test.ts | 733 ++++++++++++++++-- server/tests/unit/nest/trips.service.test.ts | 6 +- .../plugins/plugin-host-deps.factory.test.ts | 1 + .../unit/services/airtrailWriteGate.test.ts | 4 +- .../services/reservationTravelers.test.ts | 121 --- 34 files changed, 1807 insertions(+), 1175 deletions(-) delete mode 100644 server/src/mcp/tools/reservations.ts create mode 100644 server/src/nest/reservations/reservations.bridge.ts create mode 100644 server/src/nest/reservations/reservations.mcp.ts delete mode 100644 server/src/services/reservationService.ts delete mode 100644 server/tests/unit/services/reservationTravelers.test.ts diff --git a/server/src/mcp/resources.ts b/server/src/mcp/resources.ts index 35d3b989c..b7fd44dbf 100644 --- a/server/src/mcp/resources.ts +++ b/server/src/mcp/resources.ts @@ -4,7 +4,6 @@ import { listTrips, getTrip, getTripOwner, listMembers } from '../services/tripS import { listDays, listAccommodations } from '../services/dayService'; import { listPlaces } from '../services/placeService'; import { listBudgetItems, getPerPersonSummary, calculateSettlement } from '../services/budgetService'; -import { listReservations } from '../services/reservationService'; import { listBucketList, listVisitedCountries, getStats as getAtlasStats, listManuallyVisitedRegions } from '../services/atlasService'; import { getNotifications } from '../services/inAppNotifications'; import { isAddonEnabled } from '../services/adminService'; @@ -116,18 +115,8 @@ export function registerResources(server: McpServer, userId: number, scopes: str // The trip-packing resource moved to the DI-discovered // src/nest/packing/packing.mcp.ts (@ResourceTemplate). - // Reservations (flights, hotels, restaurants) - if (canRead(scopes, 'reservations')) server.registerResource( - 'trip-reservations', - new ResourceTemplate('trek://trips/{tripId}/reservations', { list: undefined }), - { description: 'Reservations (flights, hotels, restaurants) for a trip', mimeType: 'application/json' }, - async (uri, { tripId }) => { - const id = parseId(tripId); - if (id === null || !canAccessTrip(id, userId)) return accessDenied(uri.href); - const reservations = listReservations(id); - return jsonContent(uri.href, reservations); - } - ); + // The trip-reservations resource moved to the DI-discovered + // src/nest/reservations/reservations.mcp.ts (@ResourceTemplate). // The day-notes resource moved to the DI-discovered // src/nest/days/day-notes.mcp.ts (@ResourceTemplate). diff --git a/server/src/mcp/tools.ts b/server/src/mcp/tools.ts index 56cc97738..78c129adb 100644 --- a/server/src/mcp/tools.ts +++ b/server/src/mcp/tools.ts @@ -1,6 +1,5 @@ import { McpServer } from '@modelcontextprotocol/sdk/server/mcp'; import { registerJourneyTools } from './tools/journey'; -import { registerReservationTools } from './tools/reservations'; import { registerMapsWeatherTools } from './tools/mapsWeather'; import { registerNotificationTools } from './tools/notifications'; import { registerAtlasTools } from './tools/atlas'; @@ -26,7 +25,8 @@ export function registerTools(server: McpServer, userId: number, scopes: string[ // The packing tools moved to the DI-discovered src/nest/packing/packing.mcp.ts // (@McpController, attached via the nest-mcp registry below). - registerReservationTools(server, userId, scopes); + // The reservation tools moved to the DI-discovered src/nest/reservations/ + // reservations.mcp.ts (@McpController, attached via the nest-mcp registry below). registerDayTools(server, userId, scopes); diff --git a/server/src/mcp/tools/reservations.ts b/server/src/mcp/tools/reservations.ts deleted file mode 100644 index 44337c313..000000000 --- a/server/src/mcp/tools/reservations.ts +++ /dev/null @@ -1,227 +0,0 @@ -import { McpServer } from '@modelcontextprotocol/sdk/server/mcp'; -import { z } from 'zod'; -import { canAccessTrip } from '../../db/database'; -import { isDemoUser } from '../../services/authService'; -import { - createReservation, getReservation, updateReservation, deleteReservation, - updatePositions as updateReservationPositions, -} from '../../services/reservationService'; -import { linkBudgetItemToReservation } from '../../services/budgetService'; -import { getDay } from '../../services/dayService'; -import { placeExists, getAssignmentForTrip } from '../../nest/assignments/assignments.bridge'; -import { - safeBroadcast, TOOL_ANNOTATIONS_WRITE, TOOL_ANNOTATIONS_DELETE, - TOOL_ANNOTATIONS_NON_IDEMPOTENT, - demoDenied, noAccess, ok, hasTripPermission, permissionDenied, -} from './_shared'; -import { canWrite } from '../scopes'; - -export function registerReservationTools(server: McpServer, userId: number, scopes: string[] | null): void { - if (!canWrite(scopes, 'reservations')) return; - - - server.registerTool( - 'create_reservation', - { - description: 'Recommend a reservation for a trip. Created as pending — the user must confirm it. For flights, trains, cars, and cruises, use create_transport instead. Linking: hotel → use place_id + start_day_id + end_day_id (all three required to create the accommodation link); restaurant/event/tour/activity/parking/other → use assignment_id. Set price to record the cost; it will appear on the booking and in the Budget tab.', - inputSchema: { - tripId: z.number().int().positive(), - title: z.string().min(1).max(200), - type: z.enum(['hotel', 'restaurant', 'event', 'tour', 'activity', 'parking', 'other']).describe('Reservation type: "hotel", "restaurant", "event", "tour", "activity", "parking", or "other"'), - reservation_time: z.string().optional().describe('ISO 8601 datetime or time string'), - location: z.string().max(500).optional(), - confirmation_number: z.string().max(100).optional(), - notes: z.string().max(1000).optional(), - day_id: z.number().int().positive().optional(), - place_id: z.number().int().positive().optional().describe('Hotel place to link (hotel type only)'), - start_day_id: z.number().int().positive().optional().describe('Check-in day (hotel type only; requires place_id and end_day_id)'), - end_day_id: z.number().int().positive().optional().describe('Check-out day (hotel type only; requires place_id and start_day_id)'), - check_in: z.string().max(10).optional().describe('Check-in time (e.g. "15:00", hotel type only)'), - check_out: z.string().max(10).optional().describe('Check-out time (e.g. "11:00", hotel type only)'), - assignment_id: z.number().int().positive().optional().describe('Link to a day assignment (restaurant, train, car, cruise, event, tour, activity, other)'), - price: z.number().nonnegative().optional().describe('Reservation cost — shown on the booking and linked in the Budget tab'), - budget_category: z.string().max(100).optional().describe('Budget category for the price entry (defaults to reservation type)'), - }, - annotations: TOOL_ANNOTATIONS_NON_IDEMPOTENT, - }, - async ({ tripId, title, type, reservation_time, location, confirmation_number, notes, day_id, place_id, start_day_id, end_day_id, check_in, check_out, assignment_id, price, budget_category }) => { - if (isDemoUser(userId)) return demoDenied(); - if (!canAccessTrip(tripId, userId)) return noAccess(); - if (!hasTripPermission('reservation_edit', tripId, userId)) return permissionDenied(); - - // Validate that all referenced IDs belong to this trip - if (day_id && !getDay(day_id, tripId)) - return { content: [{ type: 'text' as const, text: 'day_id does not belong to this trip.' }], isError: true }; - if (place_id && !placeExists(place_id, tripId)) - return { content: [{ type: 'text' as const, text: 'place_id does not belong to this trip.' }], isError: true }; - if (start_day_id && !getDay(start_day_id, tripId)) - return { content: [{ type: 'text' as const, text: 'start_day_id does not belong to this trip.' }], isError: true }; - if (end_day_id && !getDay(end_day_id, tripId)) - return { content: [{ type: 'text' as const, text: 'end_day_id does not belong to this trip.' }], isError: true }; - if (assignment_id && !getAssignmentForTrip(assignment_id, tripId)) - return { content: [{ type: 'text' as const, text: 'assignment_id does not belong to this trip.' }], isError: true }; - - const createAccommodation = (type === 'hotel' && place_id && start_day_id && end_day_id) - ? { place_id, start_day_id, end_day_id, check_in: check_in || undefined, check_out: check_out || undefined, confirmation: confirmation_number || undefined } - : undefined; - - const metadata = price != null ? { price: String(price) } : undefined; - - const { reservation, accommodationCreated } = createReservation(tripId, { - title, type, reservation_time, location, confirmation_number, - notes, day_id, place_id, assignment_id, - create_accommodation: createAccommodation, - metadata, - }); - - if (accommodationCreated) { - safeBroadcast(tripId, 'accommodation:created', {}); - } - - if (price != null && price > 0) { - const item = linkBudgetItemToReservation(tripId, reservation.id, { - name: title, - category: budget_category || type, - total_price: price, - }); - safeBroadcast(tripId, 'budget:created', { item }); - } - - safeBroadcast(tripId, 'reservation:created', { reservation }); - return ok({ reservation }); - } - ); - - server.registerTool( - 'update_reservation', - { - description: 'Update an existing reservation in a trip. Use status "confirmed" to confirm a pending recommendation, or "pending" to revert it. For flights, trains, cars, and cruises, use update_transport instead. Linking: hotel → use place_id to link to an accommodation place; restaurant/event/tour/activity/parking/other → use assignment_id to link to a day assignment.', - inputSchema: { - tripId: z.number().int().positive(), - reservationId: z.number().int().positive(), - title: z.string().min(1).max(200).optional(), - type: z.enum(['hotel', 'restaurant', 'event', 'tour', 'activity', 'parking', 'other']).optional().describe('Reservation type: "hotel", "restaurant", "event", "tour", "activity", "parking", or "other"'), - reservation_time: z.string().optional().describe('ISO 8601 datetime or time string'), - location: z.string().max(500).optional(), - confirmation_number: z.string().max(100).optional(), - notes: z.string().max(1000).optional(), - status: z.enum(['pending', 'confirmed', 'cancelled']).optional().describe('Reservation status: "pending", "confirmed", or "cancelled"'), - place_id: z.number().int().positive().nullable().optional().describe('Link to a place (use for hotel type), or null to unlink'), - assignment_id: z.number().int().positive().nullable().optional().describe('Link to a day assignment (use for restaurant, train, car, cruise, event, tour, activity, other), or null to unlink'), - }, - annotations: TOOL_ANNOTATIONS_WRITE, - }, - async ({ tripId, reservationId, title, type, reservation_time, location, confirmation_number, notes, status, place_id, assignment_id }) => { - if (isDemoUser(userId)) return demoDenied(); - if (!canAccessTrip(tripId, userId)) return noAccess(); - if (!hasTripPermission('reservation_edit', tripId, userId)) return permissionDenied(); - const existing = getReservation(reservationId, tripId); - if (!existing) return { content: [{ type: 'text' as const, text: 'Reservation not found.' }], isError: true }; - - if (place_id != null && !placeExists(place_id, tripId)) - return { content: [{ type: 'text' as const, text: 'place_id does not belong to this trip.' }], isError: true }; - if (assignment_id != null && !getAssignmentForTrip(assignment_id, tripId)) - return { content: [{ type: 'text' as const, text: 'assignment_id does not belong to this trip.' }], isError: true }; - - const { reservation } = updateReservation(reservationId, tripId, { - title, type, reservation_time, location, confirmation_number, notes, status, - place_id: place_id !== undefined ? place_id ?? undefined : undefined, - assignment_id: assignment_id !== undefined ? assignment_id ?? undefined : undefined, - }, existing); - safeBroadcast(tripId, 'reservation:updated', { reservation }); - return ok({ reservation }); - } - ); - - server.registerTool( - 'delete_reservation', - { - description: 'Delete a reservation from a trip.', - inputSchema: { - tripId: z.number().int().positive(), - reservationId: z.number().int().positive(), - }, - annotations: TOOL_ANNOTATIONS_DELETE, - }, - async ({ tripId, reservationId }) => { - if (isDemoUser(userId)) return demoDenied(); - if (!canAccessTrip(tripId, userId)) return noAccess(); - if (!hasTripPermission('reservation_edit', tripId, userId)) return permissionDenied(); - const { deleted, accommodationDeleted } = deleteReservation(reservationId, tripId); - if (!deleted) return { content: [{ type: 'text' as const, text: 'Reservation not found.' }], isError: true }; - if (accommodationDeleted) { - safeBroadcast(tripId, 'accommodation:deleted', { accommodationId: deleted.accommodation_id }); - } - safeBroadcast(tripId, 'reservation:deleted', { reservationId }); - return ok({ success: true }); - } - ); - - server.registerTool( - 'reorder_reservations', - { - description: 'Update the display order of reservations within a day.', - inputSchema: { - tripId: z.number().int().positive(), - positions: z.array(z.object({ - id: z.number().int().positive(), - day_plan_position: z.number().int().min(0), - })).describe('Array of { id, day_plan_position } pairs'), - dayId: z.number().int().positive().optional().describe('Optionally scope the update to a specific day'), - }, - annotations: TOOL_ANNOTATIONS_WRITE, - }, - async ({ tripId, positions, dayId }) => { - if (isDemoUser(userId)) return demoDenied(); - if (!canAccessTrip(tripId, userId)) return noAccess(); - if (!hasTripPermission('reservation_edit', tripId, userId)) return permissionDenied(); - updateReservationPositions(tripId, positions, dayId); - safeBroadcast(tripId, 'reservation:positions', { positions, dayId }); - return ok({ success: true }); - } - ); - - server.registerTool( - 'link_hotel_accommodation', - { - description: 'Set or update the check-in/check-out day links for a hotel reservation. Creates or updates the accommodation record that ties the reservation to a place and a date range. Use the day IDs from get_trip_summary.', - inputSchema: { - tripId: z.number().int().positive(), - reservationId: z.number().int().positive(), - place_id: z.number().int().positive().describe('The hotel place to link'), - start_day_id: z.number().int().positive().describe('Check-in day ID'), - end_day_id: z.number().int().positive().describe('Check-out day ID'), - check_in: z.string().max(10).optional().describe('Check-in time (e.g. "15:00")'), - check_out: z.string().max(10).optional().describe('Check-out time (e.g. "11:00")'), - }, - annotations: TOOL_ANNOTATIONS_WRITE, - }, - async ({ tripId, reservationId, place_id, start_day_id, end_day_id, check_in, check_out }) => { - if (isDemoUser(userId)) return demoDenied(); - if (!canAccessTrip(tripId, userId)) return noAccess(); - if (!hasTripPermission('reservation_edit', tripId, userId)) return permissionDenied(); - const current = getReservation(reservationId, tripId); - if (!current) return { content: [{ type: 'text' as const, text: 'Reservation not found.' }], isError: true }; - if (current.type !== 'hotel') return { content: [{ type: 'text' as const, text: 'Reservation is not of type hotel.' }], isError: true }; - - if (!placeExists(place_id, tripId)) - return { content: [{ type: 'text' as const, text: 'place_id does not belong to this trip.' }], isError: true }; - if (!getDay(start_day_id, tripId)) - return { content: [{ type: 'text' as const, text: 'start_day_id does not belong to this trip.' }], isError: true }; - if (!getDay(end_day_id, tripId)) - return { content: [{ type: 'text' as const, text: 'end_day_id does not belong to this trip.' }], isError: true }; - - const isNewAccommodation = !current.accommodation_id; - const { reservation } = updateReservation(reservationId, tripId, { - place_id, - type: current.type, - status: current.status as string, - create_accommodation: { place_id, start_day_id, end_day_id, check_in: check_in || undefined, check_out: check_out || undefined }, - }, current); - - safeBroadcast(tripId, isNewAccommodation ? 'accommodation:created' : 'accommodation:updated', {}); - safeBroadcast(tripId, 'reservation:updated', { reservation }); - return ok({ reservation, accommodation_id: (reservation as any)?.accommodation_id ?? null }); - } - ); -} diff --git a/server/src/mcp/tools/transit.ts b/server/src/mcp/tools/transit.ts index 467bc1c5c..a25bb2d6e 100644 --- a/server/src/mcp/tools/transit.ts +++ b/server/src/mcp/tools/transit.ts @@ -2,7 +2,7 @@ import { canAccessTrip } from '../../db/database'; import { RateLimitService } from '../../nest/auth/rate-limit.service'; import { isDemoUser } from '../../services/authService'; import { getDay, listDays } from '../../services/dayService'; -import { createReservation, notifyBookingChange } from '../../services/reservationService'; +import { createReservation, notifyBookingChange } from '../../nest/reservations/reservations.bridge'; import { buildTransitReservationParts, cleanTransitItineraryNames, diff --git a/server/src/mcp/tools/transports.ts b/server/src/mcp/tools/transports.ts index 9a6d5a17d..38fea052d 100644 --- a/server/src/mcp/tools/transports.ts +++ b/server/src/mcp/tools/transports.ts @@ -5,7 +5,7 @@ import { isDemoUser } from '../../services/authService'; import { createReservation, deleteReservation, getReservation, updateReservation, type EndpointInput, -} from '../../services/reservationService'; +} from '../../nest/reservations/reservations.bridge'; import { linkBudgetItemToReservation } from '../../services/budgetService'; import { getDay } from '../../services/dayService'; import { findByIata } from '../../services/airportService'; diff --git a/server/src/nest/README.md b/server/src/nest/README.md index 4b92030eb..f40125d4f 100644 --- a/server/src/nest/README.md +++ b/server/src/nest/README.md @@ -28,7 +28,7 @@ mount to Nest and leaves the sibling trip routes (days, places, ...) on Express. - **Phase 2 (trip sub-domains):** vacay (addon), packing, todo. - **DI-native services (legacy `src/services/*` deleted):** tags, categories, todo, packing, day-notes, trip-invite, assignments, share, settings, files, - collab, vacay — see the migration recipe below. + collab, vacay, reservations — see the migration recipe below. ## Cross-cutting Foundation pieces @@ -134,10 +134,21 @@ vacay followed (the largest MCP port yet — a 26-tool + 3-resource `vacay.mcp.ts`, including the first fixed-URI `@Resource` behind a `when` addon gate — plus the plugin-host swap and a 1-export `vacay.bridge.ts` for `tripService`'s trip-window entry shift; the DTO ratchet for its 13 -grandfathered body contracts landed as a sibling commit). +grandfathered body contracts landed as a sibling commit); reservations +followed (the residue fold: the wrapper `ReservationsService` was already +DI-native at the edge, so the 626-line legacy module folded into it — a 5-tool ++ 1-resource `reservations.mcp.ts`, the plugin host's last plain-function +reservation import (`notifyBookingChange`) swapped for the injected service, +`TripsService` and `BookingImportService` converted from function imports to +injection, and a 9-export + 3-type `reservations.bridge.ts` for the legacy +`tripService`, the airtrail import/sync pair and the still-legacy transit + +transports registrars; the DTO ratchet for its 4 grandfathered body contracts +landed as a sibling commit, which also loosened the shared positions schema to +the real wire contract — `day_plan_position` optional, pinned by RESV-006). Repeat these steps per -service (next up: reservationService, the residue fold). This is a **pure -relocation** — byte-identical +service (next up: dayService — the dependency-honest order in +`migration-graph.md` promotes it now that reservations is folded). This is a +**pure relocation** — byte-identical SQL, statuses, bodies, and error strings. The plugin RPC host is **no longer a bridge consumer**: since Option A of `src/nest/plugins/DI-MIGRATION.md` it injects domain services via `PluginHostDepsFactory`, so a migrated domain adds diff --git a/server/src/nest/booking-import/booking-import.module.ts b/server/src/nest/booking-import/booking-import.module.ts index e4f744019..533f4c535 100644 --- a/server/src/nest/booking-import/booking-import.module.ts +++ b/server/src/nest/booking-import/booking-import.module.ts @@ -5,9 +5,10 @@ import { ImportJobsService } from './import-jobs.service'; import { KitineraryExtractorService } from './kitinerary-extractor.service'; import { FeaturesController } from './features.controller'; import { LlmParseModule } from '../llm-parse/llm-parse.module'; +import { ReservationsModule } from '../reservations/reservations.module'; @Module({ - imports: [LlmParseModule], + imports: [LlmParseModule, ReservationsModule], controllers: [BookingImportController, FeaturesController], providers: [BookingImportService, KitineraryExtractorService, ImportJobsService], }) diff --git a/server/src/nest/booking-import/booking-import.service.ts b/server/src/nest/booking-import/booking-import.service.ts index a11f2eb47..d4aedd2cd 100644 --- a/server/src/nest/booking-import/booking-import.service.ts +++ b/server/src/nest/booking-import/booking-import.service.ts @@ -2,7 +2,7 @@ import { Injectable, HttpException } from '@nestjs/common'; import { broadcast } from '../../websocket'; import { checkPermission } from '../../services/permissions'; import { verifyTripAccess } from '../../services/tripAccess'; -import { createReservation } from '../../services/reservationService'; +import { ReservationsService } from '../reservations/reservations.service'; import { createPlace } from '../../services/placeService'; import { createBudgetItem, freezeForeignRate } from '../../services/budgetService'; import { isAddonEnabled } from '../../services/adminService'; @@ -23,6 +23,7 @@ export class BookingImportService { private readonly extractor: KitineraryExtractorService, private readonly llmParse: LlmParseService, private readonly dbs: DatabaseService, + private readonly reservations: ReservationsService, ) {} private get db() { @@ -219,7 +220,7 @@ export class BookingImportService { }; } - const { reservation, accommodationCreated } = createReservation(tripId, { + const { reservation, accommodationCreated } = this.reservations.create(tripId, { ...reservationData, place_id: placeId, create_accommodation: createAccommodation, diff --git a/server/src/nest/plugins/DI-MIGRATION.md b/server/src/nest/plugins/DI-MIGRATION.md index 1bd313edc..8b16e5a4c 100644 --- a/server/src/nest/plugins/DI-MIGRATION.md +++ b/server/src/nest/plugins/DI-MIGRATION.md @@ -230,7 +230,10 @@ none of it is throwaway. stubs (see §2). 2. **Per domain, as the service migrations proceed** (`packingService`, `dayNoteService` and `assignmentService` done 2026-07; `tripInviteService` - done 2026-07 with no factory impact — it was never imported here; next per + done 2026-07 with no factory impact — it was never imported here; + `reservationService` done 2026-07, draining the factory's last + plain-function reservation import — `notifyBookingChange` became a private + method over the already-injected `ReservationsService`; next per `src/nest/README.md`): migrate the service to DI as usual; the factory swaps one legacy import for one injected service. Optionally pilot `tags.rpc.ts` here — tags is small and 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 5f00be769..1ecbe5605 100644 --- a/server/src/nest/plugins/host/plugin-host-deps.factory.ts +++ b/server/src/nest/plugins/host/plugin-host-deps.factory.ts @@ -37,17 +37,10 @@ import { DatabaseService } from '../../database/database.service'; import { FilesService } from '../../files/files.service'; import { CollabService } from '../../collab/collab.service'; import { VacayService } from '../../vacay/vacay.service'; -import { notifyBookingChange } from '../../../services/reservationService'; import { PluginRpcHost, ForbiddenResource, BadParams } from './rpc-host'; import { appendAudit } from './plugin-audit'; import { getPluginDataDb, budgetFor } from './plugin-host-state'; -// The booking notification the REST controller sends after a create/update/delete -// is fire-and-forget, so it never blocks the plugin write. -function notifyBooking(actingUserId: number, tripId: number, booking: string, type: string): void { - notifyBookingChange(tripId, actingUserId, booking, type); -} - // A subsystem read is refused when its addon is off — parity with the app, where a // disabled addon means there is simply nothing to read (same shape as the Costs gate). function requireAddon(addonId: string, noun: string): void { @@ -177,6 +170,12 @@ export class PluginHostDepsFactory { return checkPermission(action, u.role ?? 'user', trip.user_id, userId, trip.user_id !== userId); } + // The booking notification the REST controller sends after a create/update/delete + // is fire-and-forget, so it never blocks the plugin write. + private notifyBooking(actingUserId: number, tripId: number, booking: string, type: string): void { + this.reservations.notifyBookingChange(tripId, actingUserId, booking, type); + } + create(id: string, granted: ReadonlySet, router: PluginCallRouter): PluginRpcHost { return new PluginRpcHost(id, granted, { // Resolve the data handle lazily on every access rather than capturing it once. @@ -738,7 +737,7 @@ export class PluginHostDepsFactory { const i = input as { title?: string; type?: string; create_budget_entry?: unknown }; this.reservations.syncBudgetOnCreate(String(tripId), reservation.id, i.title ?? '', i.type, i.create_budget_entry as never, undefined); broadcast(tripId, 'reservation:created', { reservation }, undefined); - notifyBooking(actingUserId, tripId, i.title ?? '', i.type ?? ''); + this.notifyBooking(actingUserId, tripId, i.title ?? '', i.type ?? ''); return reservation; }, updateReservation: (tripId, reservationId, input, actingUserId) => { @@ -750,7 +749,7 @@ export class PluginHostDepsFactory { const i = input as { title?: string; type?: string; create_budget_entry?: unknown }; this.reservations.syncBudgetOnUpdate(String(tripId), String(reservationId), i.title ?? '', i.type, cur.title, cur.type, i.create_budget_entry as never, undefined); broadcast(tripId, 'reservation:updated', { reservation }, undefined); - notifyBooking(actingUserId, tripId, i.title || cur.title, i.type || cur.type || ''); + this.notifyBooking(actingUserId, tripId, i.title || cur.title, i.type || cur.type || ''); return reservation; }, deleteReservation: (tripId, reservationId, actingUserId) => { @@ -759,7 +758,7 @@ export class PluginHostDepsFactory { if (accommodationDeleted) broadcast(tripId, 'accommodation:deleted', { accommodationId: deleted.accommodation_id }, undefined); if (deletedBudgetItemId) broadcast(tripId, 'budget:deleted', { itemId: deletedBudgetItemId }, undefined); broadcast(tripId, 'reservation:deleted', { reservationId: Number(reservationId) }, undefined); - notifyBooking(actingUserId, tripId, deleted.title, deleted.type || ''); + this.notifyBooking(actingUserId, tripId, deleted.title, deleted.type || ''); return { deleted: true }; }, // --- Packing (packing_edit). Reuses PackingService + replicates the #858 diff --git a/server/src/nest/reservations/reservations.bridge.ts b/server/src/nest/reservations/reservations.bridge.ts new file mode 100644 index 000000000..7357cd8e0 --- /dev/null +++ b/server/src/nest/reservations/reservations.bridge.ts @@ -0,0 +1,62 @@ +import { db } from '../../db/database'; +import { DatabaseService } from '../database/database.service'; +import { ReservationsService } from './reservations.service'; + +export type { EndpointInput, ReservationEndpoint, ReservationTraveler } from './reservations.service'; + +/** + * Non-Nest entry point for the reservations domain — for code running OUTSIDE + * the Nest container (the legacy tripService, the airtrail import/sync + * services, and the still-legacy transit + transports MCP registrars; the + * reservation MCP tools and resource moved to the DI-discovered + * reservations.mcp.ts, and the plugin RPC host injects ReservationsService via + * PluginHostDepsFactory). Exports only the legacy services/reservationService + * names still consumed outside the container, 1:1, so repointing a consumer is + * an import-path-only diff. Inside the container, inject ReservationsService + * instead. Delete exports here as their consumers migrate. + * + * Module-level construction is safe: `db` is the reinitialize-proof Proxy onto + * the shared better-sqlite3 singleton. + */ +const reservations = new ReservationsService(new DatabaseService(db)); + +export function listReservations(tripId: string | number) { + return reservations.list(tripId); +} + +export function loadEndpointsByTrip(tripId: string | number) { + return reservations.loadEndpointsByTrip(tripId); +} + +export function resyncReservationDays(tripId: string | number): void { + reservations.resyncReservationDays(tripId); +} + +export function createReservation(tripId: string | number, data: Parameters[1]) { + return reservations.create(tripId, data); +} + +export function getReservation(id: string | number, tripId: string | number) { + return reservations.getReservation(id, tripId); +} + +export function getReservationWithJoins(id: string | number) { + return reservations.getReservationWithJoins(id); +} + +export function updateReservation( + id: string | number, + tripId: string | number, + data: Parameters[2], + current: Parameters[3] +) { + return reservations.update(id, tripId, data, current); +} + +export function deleteReservation(id: string | number, tripId: string | number) { + return reservations.remove(id, tripId); +} + +export function notifyBookingChange(tripId: string | number, actorId: number, booking: string, type: string): void { + reservations.notifyBookingChange(tripId, actorId, booking, type); +} diff --git a/server/src/nest/reservations/reservations.controller.ts b/server/src/nest/reservations/reservations.controller.ts index de546c0dc..468f5aa1a 100644 --- a/server/src/nest/reservations/reservations.controller.ts +++ b/server/src/nest/reservations/reservations.controller.ts @@ -80,7 +80,7 @@ export class ReservationsController { } this.reservations.syncBudgetOnCreate(tripId, reservation.id, body.title, body.type, body.create_budget_entry, socketId); this.reservations.broadcast(tripId, 'reservation:created', { reservation }, socketId); - this.reservations.notifyBookingChange(tripId, user, body.title, body.type ?? ''); + this.reservations.notifyBookingChange(tripId, user.id, body.title, body.type ?? ''); return { reservation }; } @@ -127,7 +127,7 @@ export class ReservationsController { if ((reservation as any)?.external_source === 'airtrail' && (reservation as any)?.sync_enabled) { void pushReservationToAirtrail(Number((reservation as any).id), Number(tripId)).catch(() => {}); } - this.reservations.notifyBookingChange(tripId, user, body.title || cur.title, body.type || cur.type || ''); + this.reservations.notifyBookingChange(tripId, user.id, body.title || cur.title, body.type || cur.type || ''); return { reservation }; } @@ -169,7 +169,7 @@ export class ReservationsController { this.reservations.broadcast(tripId, 'budget:deleted', { itemId: deletedBudgetItemId }, socketId); } this.reservations.broadcast(tripId, 'reservation:deleted', { reservationId: Number(id) }, socketId); - this.reservations.notifyBookingChange(tripId, user, deleted.title, deleted.type || ''); + this.reservations.notifyBookingChange(tripId, user.id, deleted.title, deleted.type || ''); return { success: true }; } } diff --git a/server/src/nest/reservations/reservations.mcp.ts b/server/src/nest/reservations/reservations.mcp.ts new file mode 100644 index 000000000..e13f6553b --- /dev/null +++ b/server/src/nest/reservations/reservations.mcp.ts @@ -0,0 +1,285 @@ +import { + McpController, Tool, ResourceTemplate, type McpContext, + TOOL_ANNOTATIONS_WRITE, TOOL_ANNOTATIONS_DELETE, TOOL_ANNOTATIONS_NON_IDEMPOTENT, + demoDenied, errorResult, ok, +} from '@trek/nest-mcp'; +import { z } from 'zod'; +import { isDemoUser } from '../../services/authService'; +import { linkBudgetItemToReservation } from '../../services/budgetService'; +import { getDay } from '../../services/dayService'; +import { placeExists, getAssignmentForTrip } from '../assignments/assignments.bridge'; +import { safeBroadcast, noAccess, hasTripPermission, permissionDenied } from '../../mcp/tools/_shared'; +import { ReservationsService } from './reservations.service'; + +function parseId(value: string | string[]): number | null { + const n = Number(Array.isArray(value) ? value[0] : value); + return Number.isInteger(n) && n > 0 ? n : null; +} + +/** + * Reservations MCP surface — ported 1:1 from the legacy registrars: the five + * tools from src/mcp/tools/reservations.ts and the + * trek://trips/{tripId}/reservations resource from src/mcp/resources.ts + * (identical names, descriptions, schemas, annotations, error/payload shapes + * and broadcasts). The registration-time gates map to the declarative + * reservations read/write access markers (the legacy `if (canWrite)` / + * `if (canRead)` checks, resolved by trekMcpAccessPolicy); there is no addon + * gate, so no `when`. + */ +@McpController() +export class ReservationsMcp { + constructor(private readonly reservations: ReservationsService) {} + + @Tool({ + name: 'create_reservation', + description: 'Recommend a reservation for a trip. Created as pending — the user must confirm it. For flights, trains, cars, and cruises, use create_transport instead. Linking: hotel → use place_id + start_day_id + end_day_id (all three required to create the accommodation link); restaurant/event/tour/activity/parking/other → use assignment_id. Set price to record the cost; it will appear on the booking and in the Budget tab.', + inputSchema: { + tripId: z.number().int().positive(), + title: z.string().min(1).max(200), + type: z.enum(['hotel', 'restaurant', 'event', 'tour', 'activity', 'parking', 'other']).describe('Reservation type: "hotel", "restaurant", "event", "tour", "activity", "parking", or "other"'), + reservation_time: z.string().optional().describe('ISO 8601 datetime or time string'), + location: z.string().max(500).optional(), + confirmation_number: z.string().max(100).optional(), + notes: z.string().max(1000).optional(), + day_id: z.number().int().positive().optional(), + place_id: z.number().int().positive().optional().describe('Hotel place to link (hotel type only)'), + start_day_id: z.number().int().positive().optional().describe('Check-in day (hotel type only; requires place_id and end_day_id)'), + end_day_id: z.number().int().positive().optional().describe('Check-out day (hotel type only; requires place_id and start_day_id)'), + check_in: z.string().max(10).optional().describe('Check-in time (e.g. "15:00", hotel type only)'), + check_out: z.string().max(10).optional().describe('Check-out time (e.g. "11:00", hotel type only)'), + assignment_id: z.number().int().positive().optional().describe('Link to a day assignment (restaurant, train, car, cruise, event, tour, activity, other)'), + price: z.number().nonnegative().optional().describe('Reservation cost — shown on the booking and linked in the Budget tab'), + budget_category: z.string().max(100).optional().describe('Budget category for the price entry (defaults to reservation type)'), + }, + annotations: TOOL_ANNOTATIONS_NON_IDEMPOTENT, + access: { group: 'reservations', mode: 'write' }, + }) + async createReservation( + { tripId, title, type, reservation_time, location, confirmation_number, notes, day_id, place_id, start_day_id, end_day_id, check_in, check_out, assignment_id, price, budget_category }: { + tripId: number; title: string; type: 'hotel' | 'restaurant' | 'event' | 'tour' | 'activity' | 'parking' | 'other'; + reservation_time?: string; location?: string; confirmation_number?: string; notes?: string; + day_id?: number; place_id?: number; start_day_id?: number; end_day_id?: number; + check_in?: string; check_out?: string; assignment_id?: number; price?: number; budget_category?: string; + }, + ctx: McpContext, + ) { + if (isDemoUser(ctx.userId)) return demoDenied(); + if (!this.reservations.verifyTripAccess(tripId, ctx.userId)) return noAccess(); + if (!hasTripPermission('reservation_edit', tripId, ctx.userId)) return permissionDenied(); + + // Validate that all referenced IDs belong to this trip + if (day_id && !getDay(day_id, tripId)) + return errorResult('day_id does not belong to this trip.'); + if (place_id && !placeExists(place_id, tripId)) + return errorResult('place_id does not belong to this trip.'); + if (start_day_id && !getDay(start_day_id, tripId)) + return errorResult('start_day_id does not belong to this trip.'); + if (end_day_id && !getDay(end_day_id, tripId)) + return errorResult('end_day_id does not belong to this trip.'); + if (assignment_id && !getAssignmentForTrip(assignment_id, tripId)) + return errorResult('assignment_id does not belong to this trip.'); + + const createAccommodation = (type === 'hotel' && place_id && start_day_id && end_day_id) + ? { place_id, start_day_id, end_day_id, check_in: check_in || undefined, check_out: check_out || undefined, confirmation: confirmation_number || undefined } + : undefined; + + const metadata = price != null ? { price: String(price) } : undefined; + + const { reservation, accommodationCreated } = this.reservations.create(tripId, { + title, type, reservation_time, location, confirmation_number, + notes, day_id, place_id, assignment_id, + create_accommodation: createAccommodation, + metadata, + }); + + if (accommodationCreated) { + safeBroadcast(tripId, 'accommodation:created', {}); + } + + if (price != null && price > 0) { + const item = linkBudgetItemToReservation(tripId, reservation.id, { + name: title, + category: budget_category || type, + total_price: price, + }); + safeBroadcast(tripId, 'budget:created', { item }); + } + + safeBroadcast(tripId, 'reservation:created', { reservation }); + return ok({ reservation }); + } + + @Tool({ + name: 'update_reservation', + description: 'Update an existing reservation in a trip. Use status "confirmed" to confirm a pending recommendation, or "pending" to revert it. For flights, trains, cars, and cruises, use update_transport instead. Linking: hotel → use place_id to link to an accommodation place; restaurant/event/tour/activity/parking/other → use assignment_id to link to a day assignment.', + inputSchema: { + tripId: z.number().int().positive(), + reservationId: z.number().int().positive(), + title: z.string().min(1).max(200).optional(), + type: z.enum(['hotel', 'restaurant', 'event', 'tour', 'activity', 'parking', 'other']).optional().describe('Reservation type: "hotel", "restaurant", "event", "tour", "activity", "parking", or "other"'), + reservation_time: z.string().optional().describe('ISO 8601 datetime or time string'), + location: z.string().max(500).optional(), + confirmation_number: z.string().max(100).optional(), + notes: z.string().max(1000).optional(), + status: z.enum(['pending', 'confirmed', 'cancelled']).optional().describe('Reservation status: "pending", "confirmed", or "cancelled"'), + place_id: z.number().int().positive().nullable().optional().describe('Link to a place (use for hotel type), or null to unlink'), + assignment_id: z.number().int().positive().nullable().optional().describe('Link to a day assignment (use for restaurant, train, car, cruise, event, tour, activity, other), or null to unlink'), + }, + annotations: TOOL_ANNOTATIONS_WRITE, + access: { group: 'reservations', mode: 'write' }, + }) + async updateReservation( + { tripId, reservationId, title, type, reservation_time, location, confirmation_number, notes, status, place_id, assignment_id }: { + tripId: number; reservationId: number; title?: string; + type?: 'hotel' | 'restaurant' | 'event' | 'tour' | 'activity' | 'parking' | 'other'; + reservation_time?: string; location?: string; confirmation_number?: string; notes?: string; + status?: 'pending' | 'confirmed' | 'cancelled'; place_id?: number | null; assignment_id?: number | null; + }, + ctx: McpContext, + ) { + if (isDemoUser(ctx.userId)) return demoDenied(); + if (!this.reservations.verifyTripAccess(tripId, ctx.userId)) return noAccess(); + if (!hasTripPermission('reservation_edit', tripId, ctx.userId)) return permissionDenied(); + const existing = this.reservations.getReservation(reservationId, tripId); + if (!existing) return errorResult('Reservation not found.'); + + if (place_id != null && !placeExists(place_id, tripId)) + return errorResult('place_id does not belong to this trip.'); + if (assignment_id != null && !getAssignmentForTrip(assignment_id, tripId)) + return errorResult('assignment_id does not belong to this trip.'); + + const { reservation } = this.reservations.update(reservationId, tripId, { + title, type, reservation_time, location, confirmation_number, notes, status, + place_id: place_id !== undefined ? place_id ?? undefined : undefined, + assignment_id: assignment_id !== undefined ? assignment_id ?? undefined : undefined, + }, existing); + safeBroadcast(tripId, 'reservation:updated', { reservation }); + return ok({ reservation }); + } + + @Tool({ + name: 'delete_reservation', + description: 'Delete a reservation from a trip.', + inputSchema: { + tripId: z.number().int().positive(), + reservationId: z.number().int().positive(), + }, + annotations: TOOL_ANNOTATIONS_DELETE, + access: { group: 'reservations', mode: 'write' }, + }) + async deleteReservation({ tripId, reservationId }: { tripId: number; reservationId: number }, ctx: McpContext) { + if (isDemoUser(ctx.userId)) return demoDenied(); + if (!this.reservations.verifyTripAccess(tripId, ctx.userId)) return noAccess(); + if (!hasTripPermission('reservation_edit', tripId, ctx.userId)) return permissionDenied(); + const { deleted, accommodationDeleted } = this.reservations.remove(reservationId, tripId); + if (!deleted) return errorResult('Reservation not found.'); + if (accommodationDeleted) { + safeBroadcast(tripId, 'accommodation:deleted', { accommodationId: deleted.accommodation_id }); + } + safeBroadcast(tripId, 'reservation:deleted', { reservationId }); + return ok({ success: true }); + } + + @Tool({ + name: 'reorder_reservations', + description: 'Update the display order of reservations within a day.', + inputSchema: { + tripId: z.number().int().positive(), + positions: z.array(z.object({ + id: z.number().int().positive(), + day_plan_position: z.number().int().min(0), + })).describe('Array of { id, day_plan_position } pairs'), + dayId: z.number().int().positive().optional().describe('Optionally scope the update to a specific day'), + }, + annotations: TOOL_ANNOTATIONS_WRITE, + access: { group: 'reservations', mode: 'write' }, + }) + async reorderReservations( + { tripId, positions, dayId }: { tripId: number; positions: { id: number; day_plan_position: number }[]; dayId?: number }, + ctx: McpContext, + ) { + if (isDemoUser(ctx.userId)) return demoDenied(); + if (!this.reservations.verifyTripAccess(tripId, ctx.userId)) return noAccess(); + if (!hasTripPermission('reservation_edit', tripId, ctx.userId)) return permissionDenied(); + this.reservations.updatePositions(tripId, positions, dayId); + safeBroadcast(tripId, 'reservation:positions', { positions, dayId }); + return ok({ success: true }); + } + + @Tool({ + name: 'link_hotel_accommodation', + description: 'Set or update the check-in/check-out day links for a hotel reservation. Creates or updates the accommodation record that ties the reservation to a place and a date range. Use the day IDs from get_trip_summary.', + inputSchema: { + tripId: z.number().int().positive(), + reservationId: z.number().int().positive(), + place_id: z.number().int().positive().describe('The hotel place to link'), + start_day_id: z.number().int().positive().describe('Check-in day ID'), + end_day_id: z.number().int().positive().describe('Check-out day ID'), + check_in: z.string().max(10).optional().describe('Check-in time (e.g. "15:00")'), + check_out: z.string().max(10).optional().describe('Check-out time (e.g. "11:00")'), + }, + annotations: TOOL_ANNOTATIONS_WRITE, + access: { group: 'reservations', mode: 'write' }, + }) + async linkHotelAccommodation( + { tripId, reservationId, place_id, start_day_id, end_day_id, check_in, check_out }: { + tripId: number; reservationId: number; place_id: number; start_day_id: number; end_day_id: number; + check_in?: string; check_out?: string; + }, + ctx: McpContext, + ) { + if (isDemoUser(ctx.userId)) return demoDenied(); + if (!this.reservations.verifyTripAccess(tripId, ctx.userId)) return noAccess(); + if (!hasTripPermission('reservation_edit', tripId, ctx.userId)) return permissionDenied(); + const current = this.reservations.getReservation(reservationId, tripId); + if (!current) return errorResult('Reservation not found.'); + if (current.type !== 'hotel') return errorResult('Reservation is not of type hotel.'); + + if (!placeExists(place_id, tripId)) + return errorResult('place_id does not belong to this trip.'); + if (!getDay(start_day_id, tripId)) + return errorResult('start_day_id does not belong to this trip.'); + if (!getDay(end_day_id, tripId)) + return errorResult('end_day_id does not belong to this trip.'); + + const isNewAccommodation = !current.accommodation_id; + const { reservation } = this.reservations.update(reservationId, tripId, { + place_id, + type: current.type, + status: current.status as string, + create_accommodation: { place_id, start_day_id, end_day_id, check_in: check_in || undefined, check_out: check_out || undefined }, + }, current); + + safeBroadcast(tripId, isNewAccommodation ? 'accommodation:created' : 'accommodation:updated', {}); + safeBroadcast(tripId, 'reservation:updated', { reservation }); + return ok({ reservation, accommodation_id: reservation?.accommodation_id ?? null }); + } + + @ResourceTemplate({ + name: 'trip-reservations', + uriTemplate: 'trek://trips/{tripId}/reservations', + description: 'Reservations (flights, hotels, restaurants) for a trip', + mimeType: 'application/json', + access: { group: 'reservations', mode: 'read' }, + }) + async tripReservationsResource(uri: URL, { tripId }: { tripId: string | string[] }, ctx: McpContext) { + const id = parseId(tripId); + if (id === null || !this.reservations.verifyTripAccess(id, ctx.userId)) { + return { + contents: [{ + uri: uri.href, + mimeType: 'application/json', + text: JSON.stringify({ error: 'Trip not found or access denied' }), + }], + }; + } + const reservations = this.reservations.list(id); + return { + contents: [{ + uri: uri.href, + mimeType: 'application/json', + text: JSON.stringify(reservations, null, 2), + }], + }; + } +} diff --git a/server/src/nest/reservations/reservations.module.ts b/server/src/nest/reservations/reservations.module.ts index e26216417..b491d719d 100644 --- a/server/src/nest/reservations/reservations.module.ts +++ b/server/src/nest/reservations/reservations.module.ts @@ -1,6 +1,7 @@ import { Module } from '@nestjs/common'; import { ReservationsController } from './reservations.controller'; import { ReservationsService } from './reservations.service'; +import { ReservationsMcp } from './reservations.mcp'; import { AccommodationsController } from './accommodations.controller'; import { AccommodationsService } from './accommodations.service'; import { UpcomingReservationsController } from './upcoming-reservations.controller'; @@ -8,12 +9,13 @@ import { UpcomingReservationsController } from './upcoming-reservations.controll /** * Reservations + accommodations domain (S5 — Phase 2 trip sub-domain). * Mounts: /api/trips/:tripId/reservations, /accommodations, and the cross-trip - * /api/reservations/upcoming dashboard feed. + * /api/reservations/upcoming dashboard feed. ReservationsMcp carries the + * decorator-registered MCP tools + resource for the domain. */ @Module({ controllers: [ReservationsController, AccommodationsController, UpcomingReservationsController], - providers: [ReservationsService, AccommodationsService], - // For in-container consumers (PluginHostDepsFactory). + providers: [ReservationsService, AccommodationsService, ReservationsMcp], + // For in-container consumers (PluginHostDepsFactory, TripsService, BookingImportService). exports: [ReservationsService], }) export class ReservationsModule {} diff --git a/server/src/nest/reservations/reservations.service.ts b/server/src/nest/reservations/reservations.service.ts index a296b9459..55adaa10b 100644 --- a/server/src/nest/reservations/reservations.service.ts +++ b/server/src/nest/reservations/reservations.service.ts @@ -2,31 +2,134 @@ import { Injectable } from '@nestjs/common'; import { DatabaseService } from '../database/database.service'; import { broadcast } from '../../websocket'; import { checkPermission } from '../../services/permissions'; -import type { User } from '../../types'; -import * as svc from '../../services/reservationService'; +import { verifyTripAccess } from '../../services/tripAccess'; +import { avatarUrl } from '../../services/avatarUrl'; +import type { Reservation, User } from '../../types'; import { createBudgetItem, updateBudgetItem, deleteBudgetItem, linkBudgetItemToReservation } from '../../services/budgetService'; import { typeToCostCategory } from '@trek/shared'; -type Trip = NonNullable>; +type Trip = NonNullable>; type BudgetEntry = { total_price?: number; category?: string } | undefined; +export interface ReservationEndpoint { + id?: number; + reservation_id?: number; + role: 'from' | 'to' | 'stop'; + sequence: number; + name: string; + code: string | null; + lat: number; + lng: number; + timezone: string | null; + local_time: string | null; + local_date: string | null; +} + +export type EndpointInput = Omit & { sequence?: number }; + +// --- Travelers (#1517): trip members / named guests assigned to a booking ----- + +export interface ReservationTraveler { + user_id: number; + username: string; + avatar: string | null; + avatar_url?: string | null; + is_guest?: number | null; +} + +/** A reservation row as the joined list/get queries return it (open record — + * the legacy queries selected `r.*` and callers pass the row through; the + * named fields are the columns consumers read in a typed position). */ +export type ReservationRow = Record & { + id: number; + trip_id: number; + title: string; + status: string; + type: string; + metadata?: string | null; + accommodation_id?: number | string | null; + day_positions?: Record | null; + endpoints?: ReservationEndpoint[]; + travelers?: ReservationTraveler[]; +}; + +interface CreateAccommodation { + place_id?: number; + start_day_id?: number; + end_day_id?: number; + check_in?: string; + check_out?: string; + confirmation?: string; +} + +export interface CreateReservationData { + title: string; + reservation_time?: string; + reservation_end_time?: string; + location?: string; + confirmation_number?: string; + notes?: string; + url?: string; + day_id?: number; + end_day_id?: number; + place_id?: number; + assignment_id?: number; + status?: string; + type?: string; + accommodation_id?: number; + metadata?: unknown; + create_accommodation?: CreateAccommodation; + endpoints?: EndpointInput[]; + needs_review?: boolean; +} + +export interface UpdateReservationData { + title?: string; + reservation_time?: string; + reservation_end_time?: string; + location?: string; + confirmation_number?: string; + notes?: string; + url?: string; + day_id?: number; + end_day_id?: number | null; + place_id?: number; + assignment_id?: number; + status?: string; + type?: string; + accommodation_id?: number; + metadata?: unknown; + create_accommodation?: CreateAccommodation; + endpoints?: EndpointInput[]; + needs_review?: boolean; +} + +type AccommodationTimesMeta = { + check_in_time?: string | null; + check_in_end_time?: string | null; + check_out_time?: string | null; +}; + /** - * Thin Nest wrapper around the existing reservation service. Trip-access, the - * 'reservation_edit' permission, the SQL and the WebSocket broadcasts reuse the - * legacy code unchanged. The legacy route's budget side effects (auto-create / + * Reservations domain service — owns the reservation SQL (moved 1:1 from the + * legacy services/reservationService.ts: identical statements, the `||` + * falsy-coercion defaults, the COALESCE update semantics, the post-write + * re-selects and the un-transactioned multi-statement writes). Trip access, + * the 'reservation_edit' permission and the WebSocket broadcast keep their + * legacy call paths. The legacy route's budget side effects (auto-create / * update / delete a linked budget item) and the booking notification are * encapsulated here so the controller stays thin — behaviour is 1:1. + * Non-Nest consumers (legacy tripService, airtrail import/sync, the transit + + * transports MCP registrars) go through reservations.bridge.ts instead of + * importing this class directly; the plugin RPC host injects it via + * PluginHostDepsFactory. */ @Injectable() export class ReservationsService { - constructor(private readonly dbs: DatabaseService) {} + constructor(private readonly db: DatabaseService) {} - private get db() { - return this.dbs.connection; - } - - verifyTripAccess(tripId: string, userId: number) { - return svc.verifyTripAccess(tripId, userId); + verifyTripAccess(tripId: string | number, userId: number) { + return verifyTripAccess(tripId, userId); } canEdit(trip: Trip, user: User): boolean { @@ -37,41 +140,570 @@ export class ReservationsService { broadcast(tripId, event, payload, socketId); } - list(tripId: string) { - return svc.listReservations(tripId); + /** Fire-and-forget booking-change notification, mirroring the legacy dynamic import. */ + notifyBookingChange(tripId: string | number, actorId: number, booking: string, type: string): void { + import('../../services/notificationService') + .then(({ send }) => { + try { + const actor = this.db.get<{ email: string }>('SELECT email FROM users WHERE id = ?', actorId); + if (!actor) return; + const trip = this.db.get<{ title: string }>('SELECT title FROM trips WHERE id = ?', tripId); + send({ + event: 'booking_change', + actorId, + scope: 'trip', + targetId: Number(tripId), + params: { + trip: trip?.title || 'Untitled', + actor: actor.email, + booking, + type: type || 'booking', + tripId: String(tripId), + }, + }).catch(() => {}); + } catch { + // Notifications must never make the booking write fail. + } + }) + .catch(() => {}); } - // Cross-trip "upcoming reservations" feed (dashboard widget). Reuses the legacy - // query unchanged; the default limit (6) matches the legacy inline handler. - listUpcoming(userId: number) { - return svc.getUpcomingReservations(userId); + loadEndpointsByTrip(tripId: string | number): Map { + const rows = this.db.all(` + SELECT e.* FROM reservation_endpoints e + JOIN reservations r ON e.reservation_id = r.id + WHERE r.trip_id = ? + ORDER BY e.reservation_id, e.sequence + `, tripId); + const map = new Map(); + for (const r of rows) { + const list = map.get(r.reservation_id!) ?? []; + list.push(r); + map.set(r.reservation_id!, list); + } + return map; } - create(tripId: string, data: Parameters[1]) { - return svc.createReservation(tripId, data); + private loadEndpoints(reservationId: number): ReservationEndpoint[] { + return this.db.all( + 'SELECT * FROM reservation_endpoints WHERE reservation_id = ? ORDER BY sequence', + reservationId + ); } - updatePositions(tripId: string, positions: Parameters[1], dayId: unknown): void { - svc.updatePositions(tripId, positions, dayId as Parameters[2]); + private toTraveler(r: { user_id: number; username: string; avatar: string | null; is_guest?: number | null }): ReservationTraveler { + return { user_id: r.user_id, username: r.username, avatar: r.avatar, is_guest: r.is_guest ?? null, avatar_url: avatarUrl(r) }; } - getReservation(id: string, tripId: string) { - return svc.getReservation(id, tripId); + /** Users assignable on a trip: its members (guests included) plus the owner. */ + private assignableUserIds(tripId: string | number): Set { + const members = this.db.all<{ user_id: number }>('SELECT user_id FROM trip_members WHERE trip_id = ?', tripId); + const ids = new Set(members.map(m => m.user_id)); + const owner = this.db.get<{ user_id: number }>('SELECT user_id FROM trips WHERE id = ?', tripId); + if (owner) ids.add(owner.user_id); + return ids; } - update(id: string, tripId: string, data: Parameters[2], current: Parameters[3]) { - return svc.updateReservation(id, tripId, data, current); + loadTravelersByTrip(tripId: string | number): Map { + const rows = this.db.all(` + SELECT rt.reservation_id, rt.user_id, COALESCE(u.display_name, u.username) AS username, u.avatar, u.is_guest + FROM reservation_travelers rt + JOIN reservations r ON rt.reservation_id = r.id + JOIN users u ON rt.user_id = u.id + WHERE r.trip_id = ? + ORDER BY rt.reservation_id + `, tripId); + const map = new Map(); + for (const row of rows) { + const list = map.get(row.reservation_id) ?? []; + list.push(this.toTraveler(row)); + map.set(row.reservation_id, list); + } + return map; } - remove(id: string, tripId: string) { - return svc.deleteReservation(id, tripId); + loadTravelers(reservationId: number | string): ReservationTraveler[] { + const rows = this.db.all(` + SELECT rt.user_id, COALESCE(u.display_name, u.username) AS username, u.avatar, u.is_guest + FROM reservation_travelers rt + JOIN users u ON rt.user_id = u.id + WHERE rt.reservation_id = ? + `, reservationId); + return rows.map(r => this.toTraveler(r)); + } + + /** + * Replace a reservation's assigned travelers. Only real trip members/guests are + * accepted — ids that aren't on the trip are silently dropped so a stale client + * can't leak a cross-trip user. #1517. + */ + setReservationTravelers(reservationId: number | string, tripId: string | number, userIds: number[]): void { + const allowed = this.assignableUserIds(tripId); + const ids = [...new Set(userIds)].filter(uid => allowed.has(uid)); + this.db.run('DELETE FROM reservation_travelers WHERE reservation_id = ?', reservationId); + if (ids.length > 0) { + const insert = this.db.prepare('INSERT OR IGNORE INTO reservation_travelers (reservation_id, user_id) VALUES (?, ?)'); + for (const uid of ids) insert.run(reservationId, uid); + } } /** Assign trip members / named guests to a reservation (#1517). Null when off-trip. */ setTravelers(id: string, tripId: string, userIds: number[]) { - if (!svc.getReservation(id, tripId)) return null; - svc.setReservationTravelers(id, tripId, userIds); - return { travelers: svc.loadTravelers(id), reservation: svc.getReservationWithJoins(Number(id)) }; + if (!this.getReservation(id, tripId)) return null; + this.setReservationTravelers(id, tripId, userIds); + return { travelers: this.loadTravelers(id), reservation: this.getReservationWithJoins(Number(id)) }; + } + + // Resolve the day row whose date matches the date portion of an ISO-ish + // timestamp. Used to keep `day_id` / `end_day_id` in sync with + // `reservation_time` / `reservation_end_time` so non-transport bookings + // (tours, restaurants, events, ...) end up on the right day in the UI, + // which now filters by day_id instead of reservation_time. + private resolveDayIdFromTime( + tripId: string | number, + time: string | null | undefined, + clampToNearest = true, + ): number | null { + if (!time) return null; + const datePart = time.slice(0, 10); + if (!/^\d{4}-\d{2}-\d{2}$/.test(datePart)) return null; + const exact = this.db.get<{ id: number }>('SELECT id FROM days WHERE trip_id = ? AND date = ? LIMIT 1', tripId, datePart); + if (exact) return exact.id; + // Fallback: clamp to the nearest day in the trip so an imported booking whose + // exact date has no day row (or sits just outside the span) still lands on a day. + // Skipped by callers (e.g. resyncReservationDays) that must leave a booking whose + // date now falls outside the range untouched instead of snapping it to an edge day. + if (!clampToNearest) return null; + const nearest = this.db.get<{ id: number }>( + 'SELECT id FROM days WHERE trip_id = ? ORDER BY ABS(JULIANDAY(date) - JULIANDAY(?)) ASC, date ASC LIMIT 1', + tripId, datePart + ); + return nearest?.id ?? null; + } + + // After a trip's date range changes, generateDays positionally re-dates the day rows + // (keeping their ids), so a dated booking's day_id stays glued to a now-re-dated day and + // the booking visually shifts by the offset (#1288). Re-anchor non-hotel bookings to the + // day matching their absolute reservation_time — the same derivation create/update + // use. Only updates when a matching day exists, so a booking whose date now falls outside + // the new range is left untouched. Hotels linked to a day_accommodation are excluded here — + // resyncAccommodationDays re-anchors the accommodation span and its linked reservation; + // unlinked dated hotels (e.g. imported ones) re-anchor like any other booking. + resyncReservationDays(tripId: string | number): void { + const rows = this.db.all<{ + id: number; reservation_time: string | null; reservation_end_time: string | null; + day_id: number | null; end_day_id: number | null; + }>( + `SELECT id, reservation_time, reservation_end_time, day_id, end_day_id + FROM reservations + WHERE trip_id = ? AND (type != 'hotel' OR accommodation_id IS NULL) AND reservation_time IS NOT NULL`, + tripId + ); + const update = this.db.prepare('UPDATE reservations SET day_id = ?, end_day_id = ? WHERE id = ?'); + for (const r of rows) { + const newDayId = this.resolveDayIdFromTime(tripId, r.reservation_time, false); + if (newDayId == null) continue; + const newEndDayId = r.reservation_end_time + ? (this.resolveDayIdFromTime(tripId, r.reservation_end_time, false) ?? r.end_day_id) + : r.end_day_id; + if (newDayId !== r.day_id || newEndDayId !== r.end_day_id) { + update.run(newDayId, newEndDayId, r.id); + } + } + } + + private saveEndpoints(reservationId: number, endpoints: EndpointInput[]): void { + // Run the transaction through DatabaseService (which re-derives the + // statement from the injected connection on each call). The bridge + // instance is built over the reinitialize-proof `db` Proxy, so a + // demo-reset / restore-from-backup that closes and reinitialises the + // connection can't leave this holding a dead handle — the legacy module + // bound its transaction lazily per call for the same reason ("The + // database connection is not open"). + this.db.transaction(() => { + this.db.run('DELETE FROM reservation_endpoints WHERE reservation_id = ?', reservationId); + const insert = this.db.prepare(` + INSERT INTO reservation_endpoints (reservation_id, role, sequence, name, code, lat, lng, timezone, local_time, local_date) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + `); + // lat/lng are NOT NULL: an imported transport whose pick-up/return (or station/ + // stop) couldn't be geocoded reaches here with null coords. Skip those rows rather + // than let the INSERT throw and fail the entire booking save — the dates still live + // on reservation_time/reservation_end_time, so the booking lands on its day either way. + endpoints + .filter((e) => e.lat != null && e.lng != null) + .forEach((e, i) => { + insert.run(reservationId, e.role, e.sequence ?? i, e.name, e.code ?? null, e.lat, e.lng, e.timezone ?? null, e.local_time ?? null, e.local_date ?? null); + }); + }); + } + + list(tripId: string | number) { + const reservations = this.db.all(` + SELECT r.*, d.day_number, p.name as place_name, r.assignment_id, + ap.place_id as accommodation_place_id, acc_p.name as accommodation_name, + ap.start_day_id as accommodation_start_day_id, ap.end_day_id as accommodation_end_day_id + FROM reservations r + LEFT JOIN days d ON r.day_id = d.id + LEFT JOIN places p ON r.place_id = p.id + LEFT JOIN day_accommodations ap ON r.accommodation_id = ap.id + LEFT JOIN places acc_p ON ap.place_id = acc_p.id + WHERE r.trip_id = ? + ORDER BY r.reservation_time ASC, r.created_at ASC + `, tripId); + + const dayPositions = this.db.all<{ reservation_id: number; day_id: number; position: number }>(` + SELECT rdp.reservation_id, rdp.day_id, rdp.position + FROM reservation_day_positions rdp + JOIN reservations r ON rdp.reservation_id = r.id + WHERE r.trip_id = ? + `, tripId); + + const posMap = new Map>(); + for (const dp of dayPositions) { + if (!posMap.has(dp.reservation_id)) posMap.set(dp.reservation_id, {}); + posMap.get(dp.reservation_id)![dp.day_id] = dp.position; + } + + const endpointsMap = this.loadEndpointsByTrip(tripId); + const travelersMap = this.loadTravelersByTrip(tripId); + + for (const r of reservations) { + r.day_positions = posMap.get(r.id) || null; + r.endpoints = endpointsMap.get(r.id) || []; + r.travelers = travelersMap.get(r.id) || []; + // accommodation_id is a TEXT column; the integer FK reads back as a numeric + // string (e.g. "14.0"). Normalize to an int so clients can parse it. + r.accommodation_id = r.accommodation_id == null ? null : Math.trunc(Number(r.accommodation_id)); + } + + return reservations; + } + + /** + * Upcoming reservations across all of a user's active trips, soonest first. + * Used by the dashboard's "Upcoming reservations" widget. A reservation counts + * as upcoming when its own time is in the future, or — for timeless entries — + * when its day falls on or after today. Cancelled bookings are skipped. + * The default limit (6) matches the legacy inline handler. + */ + listUpcoming(userId: number, limit = 6) { + const today = new Date().toISOString().slice(0, 10); + const now = new Date().toISOString(); + + const reservations = this.db.all>(` + SELECT r.id, r.trip_id, r.title, r.type, r.status, r.location, + r.reservation_time, r.confirmation_number, + t.title as trip_title, t.cover_image as trip_cover, + d.date as day_date, p.name as place_name, p.image_url as place_image + FROM reservations r + JOIN trips t ON t.id = r.trip_id + LEFT JOIN trip_members tm ON tm.trip_id = t.id AND tm.user_id = ? + LEFT JOIN days d ON r.day_id = d.id + LEFT JOIN places p ON r.place_id = p.id + WHERE (t.user_id = ? OR tm.user_id IS NOT NULL) + AND t.is_archived = 0 + AND r.status != 'cancelled' + AND ( + (r.reservation_time IS NOT NULL AND r.reservation_time >= ?) + OR (r.reservation_time IS NULL AND d.date IS NOT NULL AND d.date >= ?) + ) + ORDER BY COALESCE(r.reservation_time, d.date) ASC + LIMIT ? + `, userId, userId, now, today, limit); + + return reservations; + } + + getReservationWithJoins(id: string | number) { + const row = this.db.get(` + SELECT r.*, d.day_number, p.name as place_name, r.assignment_id, + ap.place_id as accommodation_place_id, acc_p.name as accommodation_name, + ap.start_day_id as accommodation_start_day_id, ap.end_day_id as accommodation_end_day_id + FROM reservations r + LEFT JOIN days d ON r.day_id = d.id + LEFT JOIN places p ON r.place_id = p.id + LEFT JOIN day_accommodations ap ON r.accommodation_id = ap.id + LEFT JOIN places acc_p ON ap.place_id = acc_p.id + WHERE r.id = ? + `, id); + if (!row) return undefined; + row.endpoints = this.loadEndpoints(row.id); + row.travelers = this.loadTravelers(row.id); + // accommodation_id is a TEXT column; the integer FK reads back as a numeric + // string (e.g. "14.0"). Normalize to an int so clients can parse it. + row.accommodation_id = row.accommodation_id == null ? null : Math.trunc(Number(row.accommodation_id)); + return row; + } + + create(tripId: string | number, data: CreateReservationData): { reservation: ReservationRow; accommodationCreated: boolean } { + const { + title, reservation_time, reservation_end_time, location, + confirmation_number, notes, url, day_id, end_day_id, place_id, assignment_id, + status, type, accommodation_id, metadata, create_accommodation, + endpoints, needs_review + } = data; + + let accommodationCreated = false; + + // Auto-create accommodation for hotel reservations + let resolvedAccommodationId: number | null = accommodation_id || null; + if (type === 'hotel' && !resolvedAccommodationId && create_accommodation) { + const { place_id: accPlaceId, start_day_id, end_day_id, check_in, check_out, confirmation: accConf } = create_accommodation; + if (start_day_id && end_day_id) { + const accResult = this.db.run( + 'INSERT INTO day_accommodations (trip_id, place_id, start_day_id, end_day_id, check_in, check_out, confirmation) VALUES (?, ?, ?, ?, ?, ?, ?)', + tripId, accPlaceId || null, start_day_id, end_day_id, check_in || null, check_out || null, accConf || confirmation_number || null + ); + resolvedAccommodationId = Number(accResult.lastInsertRowid); + accommodationCreated = true; + } + } + + // Derive day_id / end_day_id from reservation_time when the client + // didn't explicitly set them (non-hotel bookings only — hotels store + // their date range on the linked day_accommodation). + const resolvedType = type || 'other'; + let resolvedDayId: number | null = day_id ?? null; + if (resolvedDayId == null && resolvedType !== 'hotel' && reservation_time) { + resolvedDayId = this.resolveDayIdFromTime(tripId, reservation_time); + } + let resolvedEndDayId: number | null = end_day_id ?? null; + if (resolvedEndDayId == null && resolvedType !== 'hotel' && reservation_end_time) { + resolvedEndDayId = this.resolveDayIdFromTime(tripId, reservation_end_time); + } + + const result = this.db.run(` + INSERT INTO reservations (trip_id, day_id, end_day_id, place_id, assignment_id, title, reservation_time, reservation_end_time, location, confirmation_number, notes, url, status, type, accommodation_id, metadata, needs_review) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + `, + tripId, + resolvedDayId, + resolvedEndDayId, + place_id || null, + assignment_id || null, + title, + reservation_time || null, + reservation_end_time || null, + location || null, + confirmation_number || null, + notes || null, + url || null, + status || 'pending', + resolvedType, + resolvedAccommodationId, + metadata ? JSON.stringify(metadata) : null, + needs_review ? 1 : 0 + ); + + if (endpoints && endpoints.length > 0) { + this.saveEndpoints(Number(result.lastInsertRowid), endpoints); + } + + // Sync check-in/out to accommodation if linked + if (accommodation_id && metadata) { + const meta = (typeof metadata === 'string' ? JSON.parse(metadata) : metadata) as AccommodationTimesMeta; + if (meta.check_in_time || meta.check_in_end_time || meta.check_out_time) { + this.db.run( + 'UPDATE day_accommodations SET check_in = COALESCE(?, check_in), check_in_end = COALESCE(?, check_in_end), check_out = COALESCE(?, check_out) WHERE id = ?', + meta.check_in_time || null, meta.check_in_end_time || null, meta.check_out_time || null, accommodation_id + ); + } + if (confirmation_number) { + this.db.run( + 'UPDATE day_accommodations SET confirmation = COALESCE(?, confirmation) WHERE id = ?', + confirmation_number, accommodation_id + ); + } + } + + // The row was just inserted, so the re-select can't miss (legacy typed this any). + const reservation = this.getReservationWithJoins(Number(result.lastInsertRowid))!; + return { reservation, accommodationCreated }; + } + + updatePositions(tripId: string | number, positions: { id: number; day_plan_position: number }[], dayId?: number | string | null) { + if (dayId) { + // Per-day positions for multi-day reservations + const stmt = this.db.prepare('INSERT OR REPLACE INTO reservation_day_positions (reservation_id, day_id, position) VALUES (?, ?, ?)'); + this.db.transaction(() => { + for (const item of positions) { + stmt.run(item.id, dayId, item.day_plan_position); + } + }); + } else { + // Legacy: update global position + const stmt = this.db.prepare('UPDATE reservations SET day_plan_position = ? WHERE id = ? AND trip_id = ?'); + this.db.transaction(() => { + for (const item of positions) { + stmt.run(item.day_plan_position, item.id, tripId); + } + }); + } + } + + getReservation(id: string | number, tripId: string | number) { + return this.db.get('SELECT * FROM reservations WHERE id = ? AND trip_id = ?', id, tripId); + } + + update(id: string | number, tripId: string | number, data: UpdateReservationData, current: Reservation): { reservation: ReservationRow; accommodationChanged: boolean } { + const { + title, reservation_time, reservation_end_time, location, + confirmation_number, notes, url, day_id, end_day_id, place_id, assignment_id, + status, type, accommodation_id, metadata, create_accommodation, + endpoints, needs_review + } = data; + + let accommodationChanged = false; + + // Update or create accommodation for hotel reservations + let resolvedAccId: number | null = accommodation_id !== undefined ? (accommodation_id || null) : (current.accommodation_id ?? null); + if (resolvedAccId) { + const accExists = this.db.get('SELECT id FROM day_accommodations WHERE id = ?', resolvedAccId); + if (!accExists) resolvedAccId = null; + } + if (type === 'hotel' && create_accommodation) { + const { place_id: accPlaceId, start_day_id, end_day_id, check_in, check_out, confirmation: accConf } = create_accommodation; + if (start_day_id && end_day_id) { + if (resolvedAccId) { + this.db.run( + 'UPDATE day_accommodations SET place_id = ?, start_day_id = ?, end_day_id = ?, check_in = ?, check_out = ?, confirmation = ? WHERE id = ?', + accPlaceId || null, start_day_id, end_day_id, check_in || null, check_out || null, accConf || confirmation_number || null, resolvedAccId + ); + } else if (accPlaceId) { + const accResult = this.db.run( + 'INSERT INTO day_accommodations (trip_id, place_id, start_day_id, end_day_id, check_in, check_out, confirmation) VALUES (?, ?, ?, ?, ?, ?, ?)', + tripId, accPlaceId, start_day_id, end_day_id, check_in || null, check_out || null, accConf || confirmation_number || null + ); + resolvedAccId = Number(accResult.lastInsertRowid); + } + accommodationChanged = true; + } + } + + const resolvedType = (type ?? current.type) || 'other'; + const nextReservationTime = resolvedType === 'hotel' + ? null + : (reservation_time !== undefined ? (reservation_time || null) : current.reservation_time); + const nextReservationEndTime = resolvedType === 'hotel' + ? null + : (reservation_end_time !== undefined ? (reservation_end_time || null) : current.reservation_end_time); + + // day_id / end_day_id: honour an explicit value from the client, + // otherwise derive from the (possibly updated) reservation_time so the + // planner renders the booking on the correct day. + let nextDayId: number | null; + if (day_id != null) { + // Explicit day from the client (e.g. moved on the planner). + nextDayId = day_id; + } else if (resolvedType !== 'hotel' && nextReservationTime) { + // No day set but we have a date — pin it to the matching day so the booking + // still shows in the Plan (covers bookings saved without a selected day, and + // the case where an earlier edit cleared day_id). + nextDayId = this.resolveDayIdFromTime(tripId, nextReservationTime); + } else if (day_id === undefined) { + // Field absent and nothing to derive from — keep whatever it had. + nextDayId = current.day_id ?? null; + } else { + nextDayId = null; + } + + let nextEndDayId: number | null; + if (end_day_id !== undefined) { + nextEndDayId = end_day_id ?? null; + } else if (reservation_end_time !== undefined && resolvedType !== 'hotel') { + nextEndDayId = this.resolveDayIdFromTime(tripId, nextReservationEndTime); + } else { + nextEndDayId = current.end_day_id ?? null; + } + + this.db.run(` + UPDATE reservations SET + title = COALESCE(?, title), + reservation_time = ?, + reservation_end_time = ?, + location = ?, + confirmation_number = ?, + notes = ?, + url = ?, + day_id = ?, + end_day_id = ?, + place_id = ?, + assignment_id = ?, + status = COALESCE(?, status), + type = COALESCE(?, type), + accommodation_id = ?, + metadata = ?, + needs_review = COALESCE(?, needs_review) + WHERE id = ? + `, + title || null, + nextReservationTime, + nextReservationEndTime, + location !== undefined ? (location || null) : current.location, + confirmation_number !== undefined ? (confirmation_number || null) : current.confirmation_number, + notes !== undefined ? (notes || null) : current.notes, + url !== undefined ? (url || null) : (current as Reservation & { url?: string | null }).url, + nextDayId, + nextEndDayId, + place_id !== undefined ? (place_id || null) : current.place_id, + assignment_id !== undefined ? (assignment_id || null) : current.assignment_id, + status || null, + type || null, + resolvedAccId, + metadata !== undefined ? (metadata ? JSON.stringify(metadata) : null) : current.metadata, + needs_review === undefined ? null : (needs_review ? 1 : 0), + id + ); + + if (endpoints !== undefined) { + this.saveEndpoints(Number(id), endpoints); + } + + // Sync check-in/out to accommodation if linked + const resolvedMeta = metadata !== undefined ? metadata : (current.metadata ? JSON.parse(current.metadata as string) : null); + if (resolvedAccId && resolvedMeta) { + const meta = (typeof resolvedMeta === 'string' ? JSON.parse(resolvedMeta) : resolvedMeta) as AccommodationTimesMeta; + if (meta.check_in_time || meta.check_in_end_time || meta.check_out_time) { + this.db.run( + 'UPDATE day_accommodations SET check_in = COALESCE(?, check_in), check_in_end = COALESCE(?, check_in_end), check_out = COALESCE(?, check_out) WHERE id = ?', + meta.check_in_time || null, meta.check_in_end_time || null, meta.check_out_time || null, resolvedAccId + ); + } + const resolvedConf = confirmation_number !== undefined ? confirmation_number : current.confirmation_number; + if (resolvedConf) { + this.db.run( + 'UPDATE day_accommodations SET confirmation = COALESCE(?, confirmation) WHERE id = ?', + resolvedConf, resolvedAccId + ); + } + } + + // The caller passed the pre-checked `current` row, so the re-select can't + // miss (legacy typed this any). + const reservation = this.getReservationWithJoins(id)!; + return { reservation, accommodationChanged }; + } + + remove(id: string | number, tripId: string | number): { deleted: { id: number; title: string; type: string; accommodation_id: number | null } | undefined; accommodationDeleted: boolean; deletedBudgetItemId: number | null } { + const reservation = this.db.get<{ id: number; title: string; type: string; accommodation_id: number | null }>( + 'SELECT id, title, type, accommodation_id FROM reservations WHERE id = ? AND trip_id = ?', id, tripId + ); + if (!reservation) return { deleted: undefined, accommodationDeleted: false, deletedBudgetItemId: null }; + + let accommodationDeleted = false; + if (reservation.accommodation_id) { + this.db.run('DELETE FROM day_accommodations WHERE id = ?', reservation.accommodation_id); + accommodationDeleted = true; + } + + const linkedBudget = this.db.get<{ id: number }>('SELECT id FROM budget_items WHERE trip_id = ? AND reservation_id = ?', tripId, id); + if (linkedBudget) { + this.db.run('DELETE FROM budget_items WHERE id = ?', linkedBudget.id); + } + + this.db.run('DELETE FROM reservations WHERE id = ?', id); + return { deleted: reservation, accommodationDeleted, deletedBudgetItemId: linkedBudget ? linkedBudget.id : null }; } /** POST side effect: auto-create a linked budget item when a price is provided. */ @@ -95,7 +727,7 @@ export class ReservationsService { // but only if it still carries the auto-derived category (so a manual pick in // the Costs editor is preserved). Runs regardless of create_budget_entry. if (type && currentType && type !== currentType) { - const linked = this.db.prepare('SELECT id, category FROM budget_items WHERE trip_id = ? AND reservation_id = ?').get(tripId, id) as { id: number; category: string } | undefined; + const linked = this.db.get<{ id: number; category: string }>('SELECT id, category FROM budget_items WHERE trip_id = ? AND reservation_id = ?', tripId, id); if (linked) { const oldCat = typeToCostCategory(currentType); const newCat = typeToCostCategory(type); @@ -113,7 +745,7 @@ export class ReservationsService { if (!(Number(entry.total_price) > 0)) { // Explicit clear (total_price 0/empty) — drop the linked item. - const linked = this.db.prepare('SELECT id FROM budget_items WHERE trip_id = ? AND reservation_id = ?').get(tripId, id) as { id: number } | undefined; + const linked = this.db.get<{ id: number }>('SELECT id FROM budget_items WHERE trip_id = ? AND reservation_id = ?', tripId, id); if (linked) { deleteBudgetItem(linked.id, tripId); broadcast(tripId, 'budget:deleted', { itemId: linked.id }, socketId); @@ -124,13 +756,13 @@ export class ReservationsService { try { const itemName = title || currentTitle; const category = entry.category || type || currentType || 'Other'; - const existing = this.db.prepare('SELECT id FROM budget_items WHERE trip_id = ? AND reservation_id = ?').get(tripId, id) as { id: number } | undefined; + const existing = this.db.get<{ id: number }>('SELECT id FROM budget_items WHERE trip_id = ? AND reservation_id = ?', tripId, id); if (existing) { const updated = updateBudgetItem(existing.id, tripId, { name: itemName, category, total_price: entry.total_price }); broadcast(tripId, 'budget:updated', { item: updated }, socketId); } else { const item = createBudgetItem(tripId, { name: itemName, category, total_price: entry.total_price }); - this.db.prepare('UPDATE budget_items SET reservation_id = ? WHERE id = ?').run(id, item.id); + this.db.run('UPDATE budget_items SET reservation_id = ? WHERE id = ?', id, item.id); item.reservation_id = Number(id); broadcast(tripId, 'budget:created', { item }, socketId); } @@ -138,9 +770,4 @@ export class ReservationsService { console.error('[reservations] Failed to create/update budget entry:', err); } } - - /** Fire-and-forget booking-change notification, mirroring the legacy dynamic import. */ - notifyBookingChange(tripId: string, actor: User, booking: string, type: string): void { - svc.notifyBookingChange(tripId, actor.id, booking, type); - } } diff --git a/server/src/nest/trips/trips.module.ts b/server/src/nest/trips/trips.module.ts index ddc97a31f..281169a6b 100644 --- a/server/src/nest/trips/trips.module.ts +++ b/server/src/nest/trips/trips.module.ts @@ -1,6 +1,7 @@ import { Module } from '@nestjs/common'; import { FilesModule } from '../files/files.module'; import { PackingModule } from '../packing/packing.module'; +import { ReservationsModule } from '../reservations/reservations.module'; import { TodoModule } from '../todo/todo.module'; import { TripsController } from './trips.controller'; import { TripsService } from './trips.service'; @@ -8,7 +9,7 @@ import { TripsService } from './trips.service'; /** Trips aggregate root (C1 — Phase 3). Uses exact strangler prefixes so it does * not capture the nested sub-domain mounts (collab, files, ...). */ @Module({ - imports: [TodoModule, PackingModule, FilesModule], + imports: [TodoModule, PackingModule, FilesModule, ReservationsModule], controllers: [TripsController], providers: [TripsService], }) diff --git a/server/src/nest/trips/trips.service.ts b/server/src/nest/trips/trips.service.ts index fdf048537..c9970fb1b 100644 --- a/server/src/nest/trips/trips.service.ts +++ b/server/src/nest/trips/trips.service.ts @@ -9,7 +9,7 @@ import { listPlaces } from '../../services/placeService'; import { PackingService } from '../packing/packing.service'; import { TodoService } from '../todo/todo.service'; import { listBudgetItems, rebaseTripCurrency } from '../../services/budgetService'; -import { listReservations } from '../../services/reservationService'; +import { ReservationsService } from '../reservations/reservations.service'; import { FilesService } from '../files/files.service'; import { searchUnsplashPhotos, getUnsplashKey } from '../../services/unsplashService'; @@ -27,6 +27,7 @@ export class TripsService { private readonly todo: TodoService, private readonly packing: PackingService, private readonly files: FilesService, + private readonly reservations: ReservationsService, ) {} private get db() { @@ -143,7 +144,7 @@ export class TripsService { packingItems: this.packing.listItems(tripId, viewerId), todoItems: this.todo.listItems(tripId), budgetItems: listBudgetItems(tripId), - reservations: listReservations(tripId), + reservations: this.reservations.list(tripId), files: this.files.listFiles(tripId, false), accommodations: listAccommodations(tripId), members: [owner, ...(members || [])].filter(Boolean), diff --git a/server/src/services/airtrail/airtrailImport.ts b/server/src/services/airtrail/airtrailImport.ts index b6e3b9833..7dc6bfe2e 100644 --- a/server/src/services/airtrail/airtrailImport.ts +++ b/server/src/services/airtrail/airtrailImport.ts @@ -1,7 +1,7 @@ import type { AirtrailImportResult } from '@trek/shared'; import { db } from '../../db/database'; import { broadcast } from '../../websocket'; -import { createReservation } from '../reservationService'; +import { createReservation } from '../../nest/reservations/reservations.bridge'; import { getAirtrailCredentials } from './airtrailService'; import { AirtrailRequestError, listFlights } from './airtrailClient'; import type { AirtrailFlightRaw } from './airtrailClient'; diff --git a/server/src/services/airtrail/airtrailSync.ts b/server/src/services/airtrail/airtrailSync.ts index 7d2d22b12..0472467e5 100644 --- a/server/src/services/airtrail/airtrailSync.ts +++ b/server/src/services/airtrail/airtrailSync.ts @@ -3,7 +3,7 @@ import { db } from '../../db/database'; import { broadcast } from '../../websocket'; import { isAddonEnabled } from '../adminService'; import { logError, logInfo } from '../auditLog'; -import { getReservation, getReservationWithJoins, updateReservation } from '../reservationService'; +import { getReservation, getReservationWithJoins, updateReservation } from '../../nest/reservations/reservations.bridge'; import { AirtrailAuthError, AirtrailCreds, diff --git a/server/src/services/reservationService.ts b/server/src/services/reservationService.ts deleted file mode 100644 index d2c4818e0..000000000 --- a/server/src/services/reservationService.ts +++ /dev/null @@ -1,626 +0,0 @@ -import { db } from '../db/database'; -import { Reservation } from '../types'; -import { avatarUrl } from './avatarUrl'; - -export { verifyTripAccess } from './tripAccess'; - -export interface ReservationEndpoint { - id?: number; - reservation_id?: number; - role: 'from' | 'to' | 'stop'; - sequence: number; - name: string; - code: string | null; - lat: number; - lng: number; - timezone: string | null; - local_time: string | null; - local_date: string | null; -} - -export type EndpointInput = Omit & { sequence?: number }; - -export function notifyBookingChange( - tripId: string | number, - actorId: number, - booking: string, - type: string, -): void { - import('./notificationService') - .then(({ send }) => { - try { - const actor = db.prepare('SELECT email FROM users WHERE id = ?').get(actorId) as { email: string } | undefined; - if (!actor) return; - const trip = db.prepare('SELECT title FROM trips WHERE id = ?').get(tripId) as { title: string } | undefined; - send({ - event: 'booking_change', - actorId, - scope: 'trip', - targetId: Number(tripId), - params: { - trip: trip?.title || 'Untitled', - actor: actor.email, - booking, - type: type || 'booking', - tripId: String(tripId), - }, - }).catch(() => {}); - } catch { - // Notifications must never make the booking write fail. - } - }) - .catch(() => {}); -} - -export function loadEndpointsByTrip(tripId: string | number): Map { - const rows = db.prepare(` - SELECT e.* FROM reservation_endpoints e - JOIN reservations r ON e.reservation_id = r.id - WHERE r.trip_id = ? - ORDER BY e.reservation_id, e.sequence - `).all(tripId) as ReservationEndpoint[]; - const map = new Map(); - for (const r of rows) { - const list = map.get(r.reservation_id!) ?? []; - list.push(r); - map.set(r.reservation_id!, list); - } - return map; -} - -function loadEndpoints(reservationId: number): ReservationEndpoint[] { - return db.prepare( - 'SELECT * FROM reservation_endpoints WHERE reservation_id = ? ORDER BY sequence' - ).all(reservationId) as ReservationEndpoint[]; -} - -// --- Travelers (#1517): trip members / named guests assigned to a booking ----- - -export interface ReservationTraveler { - user_id: number; - username: string; - avatar: string | null; - avatar_url?: string | null; - is_guest?: number | null; -} - -const toTraveler = (r: { user_id: number; username: string; avatar: string | null; is_guest?: number | null }): ReservationTraveler => - ({ user_id: r.user_id, username: r.username, avatar: r.avatar, is_guest: r.is_guest ?? null, avatar_url: avatarUrl(r) }); - -/** Users assignable on a trip: its members (guests included) plus the owner. */ -function assignableUserIds(tripId: string | number): Set { - const members = db.prepare('SELECT user_id FROM trip_members WHERE trip_id = ?').all(tripId) as { user_id: number }[]; - const ids = new Set(members.map(m => m.user_id)); - const owner = db.prepare('SELECT user_id FROM trips WHERE id = ?').get(tripId) as { user_id: number } | undefined; - if (owner) ids.add(owner.user_id); - return ids; -} - -export function loadTravelersByTrip(tripId: string | number): Map { - const rows = db.prepare(` - SELECT rt.reservation_id, rt.user_id, COALESCE(u.display_name, u.username) AS username, u.avatar, u.is_guest - FROM reservation_travelers rt - JOIN reservations r ON rt.reservation_id = r.id - JOIN users u ON rt.user_id = u.id - WHERE r.trip_id = ? - ORDER BY rt.reservation_id - `).all(tripId) as (ReservationTraveler & { reservation_id: number })[]; - const map = new Map(); - for (const row of rows) { - const list = map.get(row.reservation_id) ?? []; - list.push(toTraveler(row)); - map.set(row.reservation_id, list); - } - return map; -} - -export function loadTravelers(reservationId: number | string): ReservationTraveler[] { - const rows = db.prepare(` - SELECT rt.user_id, COALESCE(u.display_name, u.username) AS username, u.avatar, u.is_guest - FROM reservation_travelers rt - JOIN users u ON rt.user_id = u.id - WHERE rt.reservation_id = ? - `).all(reservationId) as ReservationTraveler[]; - return rows.map(toTraveler); -} - -/** - * Replace a reservation's assigned travelers. Only real trip members/guests are - * accepted — ids that aren't on the trip are silently dropped so a stale client - * can't leak a cross-trip user. #1517. - */ -export function setReservationTravelers(reservationId: number | string, tripId: string | number, userIds: number[]): void { - const allowed = assignableUserIds(tripId); - const ids = [...new Set(userIds)].filter(uid => allowed.has(uid)); - db.prepare('DELETE FROM reservation_travelers WHERE reservation_id = ?').run(reservationId); - if (ids.length > 0) { - const insert = db.prepare('INSERT OR IGNORE INTO reservation_travelers (reservation_id, user_id) VALUES (?, ?)'); - for (const uid of ids) insert.run(reservationId, uid); - } -} - -// Resolve the day row whose date matches the date portion of an ISO-ish -// timestamp. Used to keep `day_id` / `end_day_id` in sync with -// `reservation_time` / `reservation_end_time` so non-transport bookings -// (tours, restaurants, events, ...) end up on the right day in the UI, -// which now filters by day_id instead of reservation_time. -function resolveDayIdFromTime( - tripId: string | number, - time: string | null | undefined, - clampToNearest = true, -): number | null { - if (!time) return null; - const datePart = time.slice(0, 10); - if (!/^\d{4}-\d{2}-\d{2}$/.test(datePart)) return null; - const exact = db - .prepare('SELECT id FROM days WHERE trip_id = ? AND date = ? LIMIT 1') - .get(tripId, datePart) as { id: number } | undefined; - if (exact) return exact.id; - // Fallback: clamp to the nearest day in the trip so an imported booking whose - // exact date has no day row (or sits just outside the span) still lands on a day. - // Skipped by callers (e.g. resyncReservationDays) that must leave a booking whose - // date now falls outside the range untouched instead of snapping it to an edge day. - if (!clampToNearest) return null; - const nearest = db - .prepare('SELECT id FROM days WHERE trip_id = ? ORDER BY ABS(JULIANDAY(date) - JULIANDAY(?)) ASC, date ASC LIMIT 1') - .get(tripId, datePart) as { id: number } | undefined; - return nearest?.id ?? null; -} - -// After a trip's date range changes, generateDays positionally re-dates the day rows -// (keeping their ids), so a dated booking's day_id stays glued to a now-re-dated day and -// the booking visually shifts by the offset (#1288). Re-anchor non-hotel bookings to the -// day matching their absolute reservation_time — the same derivation create/updateReservation -// use. Only updates when a matching day exists, so a booking whose date now falls outside -// the new range is left untouched. Hotels linked to a day_accommodation are excluded here — -// resyncAccommodationDays re-anchors the accommodation span and its linked reservation; -// unlinked dated hotels (e.g. imported ones) re-anchor like any other booking. -export function resyncReservationDays(tripId: string | number): void { - const rows = db.prepare( - `SELECT id, reservation_time, reservation_end_time, day_id, end_day_id - FROM reservations - WHERE trip_id = ? AND (type != 'hotel' OR accommodation_id IS NULL) AND reservation_time IS NOT NULL`, - ).all(tripId) as { - id: number; reservation_time: string | null; reservation_end_time: string | null; - day_id: number | null; end_day_id: number | null; - }[]; - const update = db.prepare('UPDATE reservations SET day_id = ?, end_day_id = ? WHERE id = ?'); - for (const r of rows) { - const newDayId = resolveDayIdFromTime(tripId, r.reservation_time, false); - if (newDayId == null) continue; - const newEndDayId = r.reservation_end_time - ? (resolveDayIdFromTime(tripId, r.reservation_end_time, false) ?? r.end_day_id) - : r.end_day_id; - if (newDayId !== r.day_id || newEndDayId !== r.end_day_id) { - update.run(newDayId, newEndDayId, r.id); - } - } -} - -function saveEndpoints(reservationId: number, endpoints: EndpointInput[]): void { - // Bind the transaction lazily on each call. Binding at module load time - // captures the DB connection that was open then, which becomes invalid - // after demo-reset / restore-from-backup closes and reinitialises the - // connection — every later endpoint save would throw - // "The database connection is not open". - const tx = db.transaction((rid: number, eps: EndpointInput[]) => { - db.prepare('DELETE FROM reservation_endpoints WHERE reservation_id = ?').run(rid); - const insert = db.prepare(` - INSERT INTO reservation_endpoints (reservation_id, role, sequence, name, code, lat, lng, timezone, local_time, local_date) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?) - `); - // lat/lng are NOT NULL: an imported transport whose pick-up/return (or station/ - // stop) couldn't be geocoded reaches here with null coords. Skip those rows rather - // than let the INSERT throw and fail the entire booking save — the dates still live - // on reservation_time/reservation_end_time, so the booking lands on its day either way. - eps - .filter((e) => e.lat != null && e.lng != null) - .forEach((e, i) => { - insert.run(rid, e.role, e.sequence ?? i, e.name, e.code ?? null, e.lat, e.lng, e.timezone ?? null, e.local_time ?? null, e.local_date ?? null); - }); - }); - tx(reservationId, endpoints); -} - -export function listReservations(tripId: string | number) { - const reservations = db.prepare(` - SELECT r.*, d.day_number, p.name as place_name, r.assignment_id, - ap.place_id as accommodation_place_id, acc_p.name as accommodation_name, - ap.start_day_id as accommodation_start_day_id, ap.end_day_id as accommodation_end_day_id - FROM reservations r - LEFT JOIN days d ON r.day_id = d.id - LEFT JOIN places p ON r.place_id = p.id - LEFT JOIN day_accommodations ap ON r.accommodation_id = ap.id - LEFT JOIN places acc_p ON ap.place_id = acc_p.id - WHERE r.trip_id = ? - ORDER BY r.reservation_time ASC, r.created_at ASC - `).all(tripId) as any[]; - - const dayPositions = db.prepare(` - SELECT rdp.reservation_id, rdp.day_id, rdp.position - FROM reservation_day_positions rdp - JOIN reservations r ON rdp.reservation_id = r.id - WHERE r.trip_id = ? - `).all(tripId) as { reservation_id: number; day_id: number; position: number }[]; - - const posMap = new Map>(); - for (const dp of dayPositions) { - if (!posMap.has(dp.reservation_id)) posMap.set(dp.reservation_id, {}); - posMap.get(dp.reservation_id)![dp.day_id] = dp.position; - } - - const endpointsMap = loadEndpointsByTrip(tripId); - const travelersMap = loadTravelersByTrip(tripId); - - for (const r of reservations) { - r.day_positions = posMap.get(r.id) || null; - r.endpoints = endpointsMap.get(r.id) || []; - r.travelers = travelersMap.get(r.id) || []; - // accommodation_id is a TEXT column; the integer FK reads back as a numeric - // string (e.g. "14.0"). Normalize to an int so clients can parse it. - r.accommodation_id = r.accommodation_id == null ? null : Math.trunc(Number(r.accommodation_id)); - } - - return reservations; -} - -/** - * Upcoming reservations across all of a user's active trips, soonest first. - * Used by the dashboard's "Upcoming reservations" widget. A reservation counts - * as upcoming when its own time is in the future, or — for timeless entries — - * when its day falls on or after today. Cancelled bookings are skipped. - */ -export function getUpcomingReservations(userId: number, limit = 6) { - const today = new Date().toISOString().slice(0, 10); - const now = new Date().toISOString(); - - const reservations = db.prepare(` - SELECT r.id, r.trip_id, r.title, r.type, r.status, r.location, - r.reservation_time, r.confirmation_number, - t.title as trip_title, t.cover_image as trip_cover, - d.date as day_date, p.name as place_name, p.image_url as place_image - FROM reservations r - JOIN trips t ON t.id = r.trip_id - LEFT JOIN trip_members tm ON tm.trip_id = t.id AND tm.user_id = ? - LEFT JOIN days d ON r.day_id = d.id - LEFT JOIN places p ON r.place_id = p.id - WHERE (t.user_id = ? OR tm.user_id IS NOT NULL) - AND t.is_archived = 0 - AND r.status != 'cancelled' - AND ( - (r.reservation_time IS NOT NULL AND r.reservation_time >= ?) - OR (r.reservation_time IS NULL AND d.date IS NOT NULL AND d.date >= ?) - ) - ORDER BY COALESCE(r.reservation_time, d.date) ASC - LIMIT ? - `).all(userId, userId, now, today, limit) as any[]; - - return reservations; -} - -export function getReservationWithJoins(id: string | number) { - const row = db.prepare(` - SELECT r.*, d.day_number, p.name as place_name, r.assignment_id, - ap.place_id as accommodation_place_id, acc_p.name as accommodation_name, - ap.start_day_id as accommodation_start_day_id, ap.end_day_id as accommodation_end_day_id - FROM reservations r - LEFT JOIN days d ON r.day_id = d.id - LEFT JOIN places p ON r.place_id = p.id - LEFT JOIN day_accommodations ap ON r.accommodation_id = ap.id - LEFT JOIN places acc_p ON ap.place_id = acc_p.id - WHERE r.id = ? - `).get(id) as any; - if (!row) return undefined; - row.endpoints = loadEndpoints(row.id); - row.travelers = loadTravelers(row.id); - // accommodation_id is a TEXT column; the integer FK reads back as a numeric - // string (e.g. "14.0"). Normalize to an int so clients can parse it. - row.accommodation_id = row.accommodation_id == null ? null : Math.trunc(Number(row.accommodation_id)); - return row; -} - -interface CreateAccommodation { - place_id?: number; - start_day_id?: number; - end_day_id?: number; - check_in?: string; - check_out?: string; - confirmation?: string; -} - -interface CreateReservationData { - title: string; - reservation_time?: string; - reservation_end_time?: string; - location?: string; - confirmation_number?: string; - notes?: string; - url?: string; - day_id?: number; - end_day_id?: number; - place_id?: number; - assignment_id?: number; - status?: string; - type?: string; - accommodation_id?: number; - metadata?: any; - create_accommodation?: CreateAccommodation; - endpoints?: EndpointInput[]; - needs_review?: boolean; -} - -export function createReservation(tripId: string | number, data: CreateReservationData): { reservation: any; accommodationCreated: boolean } { - const { - title, reservation_time, reservation_end_time, location, - confirmation_number, notes, url, day_id, end_day_id, place_id, assignment_id, - status, type, accommodation_id, metadata, create_accommodation, - endpoints, needs_review - } = data; - - let accommodationCreated = false; - - // Auto-create accommodation for hotel reservations - let resolvedAccommodationId: number | null = accommodation_id || null; - if (type === 'hotel' && !resolvedAccommodationId && create_accommodation) { - const { place_id: accPlaceId, start_day_id, end_day_id, check_in, check_out, confirmation: accConf } = create_accommodation; - if (start_day_id && end_day_id) { - const accResult = db.prepare( - 'INSERT INTO day_accommodations (trip_id, place_id, start_day_id, end_day_id, check_in, check_out, confirmation) VALUES (?, ?, ?, ?, ?, ?, ?)' - ).run(tripId, accPlaceId || null, start_day_id, end_day_id, check_in || null, check_out || null, accConf || confirmation_number || null); - resolvedAccommodationId = Number(accResult.lastInsertRowid); - accommodationCreated = true; - } - } - - // Derive day_id / end_day_id from reservation_time when the client - // didn't explicitly set them (non-hotel bookings only — hotels store - // their date range on the linked day_accommodation). - const resolvedType = type || 'other'; - let resolvedDayId: number | null = day_id ?? null; - if (resolvedDayId == null && resolvedType !== 'hotel' && reservation_time) { - resolvedDayId = resolveDayIdFromTime(tripId, reservation_time); - } - let resolvedEndDayId: number | null = end_day_id ?? null; - if (resolvedEndDayId == null && resolvedType !== 'hotel' && reservation_end_time) { - resolvedEndDayId = resolveDayIdFromTime(tripId, reservation_end_time); - } - - const result = db.prepare(` - INSERT INTO reservations (trip_id, day_id, end_day_id, place_id, assignment_id, title, reservation_time, reservation_end_time, location, confirmation_number, notes, url, status, type, accommodation_id, metadata, needs_review) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) - `).run( - tripId, - resolvedDayId, - resolvedEndDayId, - place_id || null, - assignment_id || null, - title, - reservation_time || null, - reservation_end_time || null, - location || null, - confirmation_number || null, - notes || null, - url || null, - status || 'pending', - resolvedType, - resolvedAccommodationId, - metadata ? JSON.stringify(metadata) : null, - needs_review ? 1 : 0 - ); - - if (endpoints && endpoints.length > 0) { - saveEndpoints(Number(result.lastInsertRowid), endpoints); - } - - // Sync check-in/out to accommodation if linked - if (accommodation_id && metadata) { - const meta = typeof metadata === 'string' ? JSON.parse(metadata) : metadata; - if (meta.check_in_time || meta.check_in_end_time || meta.check_out_time) { - db.prepare('UPDATE day_accommodations SET check_in = COALESCE(?, check_in), check_in_end = COALESCE(?, check_in_end), check_out = COALESCE(?, check_out) WHERE id = ?') - .run(meta.check_in_time || null, meta.check_in_end_time || null, meta.check_out_time || null, accommodation_id); - } - if (confirmation_number) { - db.prepare('UPDATE day_accommodations SET confirmation = COALESCE(?, confirmation) WHERE id = ?') - .run(confirmation_number, accommodation_id); - } - } - - const reservation = getReservationWithJoins(Number(result.lastInsertRowid)); - return { reservation, accommodationCreated }; -} - -export function updatePositions(tripId: string | number, positions: { id: number; day_plan_position: number }[], dayId?: number | string) { - if (dayId) { - // Per-day positions for multi-day reservations - const stmt = db.prepare('INSERT OR REPLACE INTO reservation_day_positions (reservation_id, day_id, position) VALUES (?, ?, ?)'); - const updateMany = db.transaction((items: { id: number; day_plan_position: number }[]) => { - for (const item of items) { - stmt.run(item.id, dayId, item.day_plan_position); - } - }); - updateMany(positions); - } else { - // Legacy: update global position - const stmt = db.prepare('UPDATE reservations SET day_plan_position = ? WHERE id = ? AND trip_id = ?'); - const updateMany = db.transaction((items: { id: number; day_plan_position: number }[]) => { - for (const item of items) { - stmt.run(item.day_plan_position, item.id, tripId); - } - }); - updateMany(positions); - } -} - -export function getReservation(id: string | number, tripId: string | number) { - return db.prepare('SELECT * FROM reservations WHERE id = ? AND trip_id = ?').get(id, tripId) as Reservation | undefined; -} - -interface UpdateReservationData { - title?: string; - reservation_time?: string; - reservation_end_time?: string; - location?: string; - confirmation_number?: string; - notes?: string; - url?: string; - day_id?: number; - end_day_id?: number | null; - place_id?: number; - assignment_id?: number; - status?: string; - type?: string; - accommodation_id?: number; - metadata?: any; - create_accommodation?: CreateAccommodation; - endpoints?: EndpointInput[]; - needs_review?: boolean; -} - -export function updateReservation(id: string | number, tripId: string | number, data: UpdateReservationData, current: Reservation): { reservation: any; accommodationChanged: boolean } { - const { - title, reservation_time, reservation_end_time, location, - confirmation_number, notes, url, day_id, end_day_id, place_id, assignment_id, - status, type, accommodation_id, metadata, create_accommodation, - endpoints, needs_review - } = data; - - let accommodationChanged = false; - - // Update or create accommodation for hotel reservations - let resolvedAccId: number | null = accommodation_id !== undefined ? (accommodation_id || null) : (current.accommodation_id ?? null); - if (resolvedAccId) { - const accExists = db.prepare('SELECT id FROM day_accommodations WHERE id = ?').get(resolvedAccId); - if (!accExists) resolvedAccId = null; - } - if (type === 'hotel' && create_accommodation) { - const { place_id: accPlaceId, start_day_id, end_day_id, check_in, check_out, confirmation: accConf } = create_accommodation; - if (start_day_id && end_day_id) { - if (resolvedAccId) { - db.prepare('UPDATE day_accommodations SET place_id = ?, start_day_id = ?, end_day_id = ?, check_in = ?, check_out = ?, confirmation = ? WHERE id = ?') - .run(accPlaceId || null, start_day_id, end_day_id, check_in || null, check_out || null, accConf || confirmation_number || null, resolvedAccId); - } else if (accPlaceId) { - const accResult = db.prepare( - 'INSERT INTO day_accommodations (trip_id, place_id, start_day_id, end_day_id, check_in, check_out, confirmation) VALUES (?, ?, ?, ?, ?, ?, ?)' - ).run(tripId, accPlaceId, start_day_id, end_day_id, check_in || null, check_out || null, accConf || confirmation_number || null); - resolvedAccId = Number(accResult.lastInsertRowid); - } - accommodationChanged = true; - } - } - - const resolvedType = (type ?? current.type) || 'other'; - const nextReservationTime = resolvedType === 'hotel' - ? null - : (reservation_time !== undefined ? (reservation_time || null) : current.reservation_time); - const nextReservationEndTime = resolvedType === 'hotel' - ? null - : (reservation_end_time !== undefined ? (reservation_end_time || null) : current.reservation_end_time); - - // day_id / end_day_id: honour an explicit value from the client, - // otherwise derive from the (possibly updated) reservation_time so the - // planner renders the booking on the correct day. - let nextDayId: number | null; - if (day_id != null) { - // Explicit day from the client (e.g. moved on the planner). - nextDayId = day_id; - } else if (resolvedType !== 'hotel' && nextReservationTime) { - // No day set but we have a date — pin it to the matching day so the booking - // still shows in the Plan (covers bookings saved without a selected day, and - // the case where an earlier edit cleared day_id). - nextDayId = resolveDayIdFromTime(tripId, nextReservationTime); - } else if (day_id === undefined) { - // Field absent and nothing to derive from — keep whatever it had. - nextDayId = current.day_id ?? null; - } else { - nextDayId = null; - } - - let nextEndDayId: number | null; - if (end_day_id !== undefined) { - nextEndDayId = end_day_id ?? null; - } else if (reservation_end_time !== undefined && resolvedType !== 'hotel') { - nextEndDayId = resolveDayIdFromTime(tripId, nextReservationEndTime); - } else { - nextEndDayId = (current as any).end_day_id ?? null; - } - - db.prepare(` - UPDATE reservations SET - title = COALESCE(?, title), - reservation_time = ?, - reservation_end_time = ?, - location = ?, - confirmation_number = ?, - notes = ?, - url = ?, - day_id = ?, - end_day_id = ?, - place_id = ?, - assignment_id = ?, - status = COALESCE(?, status), - type = COALESCE(?, type), - accommodation_id = ?, - metadata = ?, - needs_review = COALESCE(?, needs_review) - WHERE id = ? - `).run( - title || null, - nextReservationTime, - nextReservationEndTime, - location !== undefined ? (location || null) : current.location, - confirmation_number !== undefined ? (confirmation_number || null) : current.confirmation_number, - notes !== undefined ? (notes || null) : current.notes, - url !== undefined ? (url || null) : (current as any).url, - nextDayId, - nextEndDayId, - place_id !== undefined ? (place_id || null) : current.place_id, - assignment_id !== undefined ? (assignment_id || null) : current.assignment_id, - status || null, - type || null, - resolvedAccId, - metadata !== undefined ? (metadata ? JSON.stringify(metadata) : null) : current.metadata, - needs_review === undefined ? null : (needs_review ? 1 : 0), - id - ); - - if (endpoints !== undefined) { - saveEndpoints(Number(id), endpoints); - } - - // Sync check-in/out to accommodation if linked - const resolvedMeta = metadata !== undefined ? metadata : (current.metadata ? JSON.parse(current.metadata as string) : null); - if (resolvedAccId && resolvedMeta) { - const meta = typeof resolvedMeta === 'string' ? JSON.parse(resolvedMeta) : resolvedMeta; - if (meta.check_in_time || meta.check_in_end_time || meta.check_out_time) { - db.prepare('UPDATE day_accommodations SET check_in = COALESCE(?, check_in), check_in_end = COALESCE(?, check_in_end), check_out = COALESCE(?, check_out) WHERE id = ?') - .run(meta.check_in_time || null, meta.check_in_end_time || null, meta.check_out_time || null, resolvedAccId); - } - const resolvedConf = confirmation_number !== undefined ? confirmation_number : current.confirmation_number; - if (resolvedConf) { - db.prepare('UPDATE day_accommodations SET confirmation = COALESCE(?, confirmation) WHERE id = ?') - .run(resolvedConf, resolvedAccId); - } - } - - const reservation = getReservationWithJoins(id); - return { reservation, accommodationChanged }; -} - -export function deleteReservation(id: string | number, tripId: string | number): { deleted: { id: number; title: string; type: string; accommodation_id: number | null } | undefined; accommodationDeleted: boolean; deletedBudgetItemId: number | null } { - const reservation = db.prepare('SELECT id, title, type, accommodation_id FROM reservations WHERE id = ? AND trip_id = ?').get(id, tripId) as { id: number; title: string; type: string; accommodation_id: number | null } | undefined; - if (!reservation) return { deleted: undefined, accommodationDeleted: false, deletedBudgetItemId: null }; - - let accommodationDeleted = false; - if (reservation.accommodation_id) { - db.prepare('DELETE FROM day_accommodations WHERE id = ?').run(reservation.accommodation_id); - accommodationDeleted = true; - } - - const linkedBudget = db.prepare('SELECT id FROM budget_items WHERE trip_id = ? AND reservation_id = ?').get(tripId, id) as { id: number } | undefined; - if (linkedBudget) { - db.prepare('DELETE FROM budget_items WHERE id = ?').run(linkedBudget.id); - } - - db.prepare('DELETE FROM reservations WHERE id = ?').run(id); - return { deleted: reservation, accommodationDeleted, deletedBudgetItemId: linkedBudget ? linkedBudget.id : null }; -} diff --git a/server/src/services/transitItineraryService.ts b/server/src/services/transitItineraryService.ts index 3e1c2bbfd..d14c74ee1 100644 --- a/server/src/services/transitItineraryService.ts +++ b/server/src/services/transitItineraryService.ts @@ -1,5 +1,5 @@ import { haversineKm } from './distanceService'; -import type { EndpointInput } from './reservationService'; +import type { EndpointInput } from '../nest/reservations/reservations.bridge'; import { localParts, resolveTimeZone } from './timezoneService'; import { deriveTransitStats, type TransitItinerary, type TransitLeg } from './transitService'; diff --git a/server/src/services/tripService.ts b/server/src/services/tripService.ts index 8eaa36221..4c9f23f02 100644 --- a/server/src/services/tripService.ts +++ b/server/src/services/tripService.ts @@ -9,7 +9,7 @@ import { Trip, User } from '../types'; import { listDays, listAccommodations, addDays, resyncAccommodationDays, restampReservationDates } from './dayService'; import { listBudgetItems, removeUserFromBudgetItems } from './budgetService'; import { listItems as listPackingItems } from '../nest/packing/packing.bridge'; -import { listReservations, loadEndpointsByTrip, resyncReservationDays } from './reservationService'; +import { listReservations, loadEndpointsByTrip, resyncReservationDays } from '../nest/reservations/reservations.bridge'; import { listNotes as listCollabNotes } from '../nest/collab/collab.bridge'; import { shiftOwnerEntriesForTripWindow } from '../nest/vacay/vacay.bridge'; import { resolveTimeZone } from './timezoneService'; diff --git a/server/tests/e2e/trips.e2e.test.ts b/server/tests/e2e/trips.e2e.test.ts index a20c1e423..f00581450 100644 --- a/server/tests/e2e/trips.e2e.test.ts +++ b/server/tests/e2e/trips.e2e.test.ts @@ -19,7 +19,7 @@ const { db } = vi.hoisted(() => { tmp.exec('PRAGMA journal_mode = WAL'); tmp.exec(`CREATE TABLE users (id INTEGER PRIMARY KEY AUTOINCREMENT, username TEXT NOT NULL, email TEXT NOT NULL UNIQUE, role TEXT NOT NULL DEFAULT 'user', password_version INTEGER NOT NULL DEFAULT 0, - avatar TEXT);`); + avatar TEXT, display_name TEXT, is_guest INTEGER NOT NULL DEFAULT 0);`); tmp.exec('CREATE TABLE trips (id INTEGER PRIMARY KEY AUTOINCREMENT, title TEXT);'); // bundle()'s todoItems now runs TodoService's real SQL (DI-injected, no mock). tmp.exec(`CREATE TABLE todo_items (id INTEGER PRIMARY KEY AUTOINCREMENT, trip_id INTEGER NOT NULL, @@ -44,7 +44,27 @@ const { db } = vi.hoisted(() => { tmp.exec(`CREATE TABLE file_links (id INTEGER PRIMARY KEY AUTOINCREMENT, file_id INTEGER NOT NULL, reservation_id INTEGER, assignment_id INTEGER, place_id INTEGER, created_at DATETIME DEFAULT CURRENT_TIMESTAMP);`); - tmp.exec('CREATE TABLE IF NOT EXISTS reservations (id INTEGER PRIMARY KEY AUTOINCREMENT, trip_id INTEGER, title TEXT);'); + // bundle()'s reservations now runs ReservationsService's real SQL + // (DI-injected, no mock) — the joined list query needs the full reservation + // table set (trimmed from src/db/schema.ts; accommodation_id is TEXT there). + tmp.exec(`CREATE TABLE reservations (id INTEGER PRIMARY KEY AUTOINCREMENT, trip_id INTEGER, title TEXT, + day_id INTEGER, end_day_id INTEGER, place_id INTEGER, assignment_id INTEGER, type TEXT, + status TEXT DEFAULT 'pending', reservation_time TEXT, reservation_end_time TEXT, location TEXT, + confirmation_number TEXT, notes TEXT, url TEXT, accommodation_id TEXT, metadata TEXT, + needs_review INTEGER DEFAULT 0, day_plan_position REAL, external_source TEXT, sync_enabled INTEGER, + created_at DATETIME DEFAULT CURRENT_TIMESTAMP);`); + tmp.exec('CREATE TABLE days (id INTEGER PRIMARY KEY AUTOINCREMENT, trip_id INTEGER NOT NULL, day_number INTEGER, date TEXT);'); + tmp.exec('CREATE TABLE places (id INTEGER PRIMARY KEY AUTOINCREMENT, trip_id INTEGER NOT NULL, name TEXT, image_url TEXT);'); + tmp.exec(`CREATE TABLE day_accommodations (id INTEGER PRIMARY KEY AUTOINCREMENT, trip_id INTEGER NOT NULL, + place_id INTEGER, start_day_id INTEGER, end_day_id INTEGER, check_in TEXT, check_in_end TEXT, + check_out TEXT, confirmation TEXT);`); + tmp.exec(`CREATE TABLE reservation_day_positions (reservation_id INTEGER NOT NULL, day_id INTEGER NOT NULL, + position REAL, PRIMARY KEY (reservation_id, day_id));`); + tmp.exec(`CREATE TABLE reservation_endpoints (id INTEGER PRIMARY KEY AUTOINCREMENT, reservation_id INTEGER NOT NULL, + role TEXT, sequence INTEGER, name TEXT, code TEXT, lat REAL NOT NULL, lng REAL NOT NULL, + timezone TEXT, local_time TEXT, local_date TEXT);`); + tmp.exec(`CREATE TABLE reservation_travelers (reservation_id INTEGER NOT NULL, user_id INTEGER NOT NULL, + PRIMARY KEY (reservation_id, user_id));`); return { db: tmp }; }); @@ -54,7 +74,7 @@ vi.mock('../../src/db/database', () => ({ })); vi.mock('../../src/websocket', () => ({ broadcast: vi.fn() })); vi.mock('../../src/services/notificationService', () => ({ send: vi.fn().mockResolvedValue(undefined) })); -vi.mock('../../src/services/auditLog', () => ({ writeAudit: vi.fn(), getClientIp: vi.fn(() => '1.2.3.4'), logInfo: vi.fn() })); +vi.mock('../../src/services/auditLog', () => ({ writeAudit: vi.fn(), getClientIp: vi.fn(() => '1.2.3.4'), logInfo: vi.fn(), logError: vi.fn() })); vi.mock('../../src/services/demo', () => ({ isDemoEmail: vi.fn(() => false) })); const { checkPermission } = vi.hoisted(() => ({ checkPermission: vi.fn() })); @@ -72,7 +92,6 @@ vi.mock('../../src/services/tripService', () => tripSvc); vi.mock('../../src/services/dayService', () => ({ listDays: () => ({ days: [] }), listAccommodations: () => [] })); vi.mock('../../src/services/placeService', () => ({ listPlaces: () => [] })); vi.mock('../../src/services/budgetService', () => ({ listBudgetItems: () => [] })); -vi.mock('../../src/services/reservationService', () => ({ listReservations: () => [] })); import { TripsModule } from '../../src/nest/trips/trips.module'; import { TrekExceptionFilter } from '../../src/nest/common/trek-exception.filter'; diff --git a/server/tests/helpers/mcp-test-controllers.ts b/server/tests/helpers/mcp-test-controllers.ts index 3e2b2c05f..c35356ce0 100644 --- a/server/tests/helpers/mcp-test-controllers.ts +++ b/server/tests/helpers/mcp-test-controllers.ts @@ -12,6 +12,8 @@ import { DayNotesMcp } from '../../src/nest/days/day-notes.mcp'; import { DayNotesService } from '../../src/nest/days/day-notes.service'; import { PackingMcp } from '../../src/nest/packing/packing.mcp'; import { PackingService } from '../../src/nest/packing/packing.service'; +import { ReservationsMcp } from '../../src/nest/reservations/reservations.mcp'; +import { ReservationsService } from '../../src/nest/reservations/reservations.service'; import { TagsMcp } from '../../src/nest/tags/tags.mcp'; import { TagsService } from '../../src/nest/tags/tags.service'; import { TodoMcp } from '../../src/nest/todo/todo.mcp'; @@ -34,6 +36,7 @@ export function createMcpTestRegistry(): McpRegistry { new CategoriesMcp(new CategoriesService(dbService)), new TodoMcp(new TodoService(dbService)), new PackingMcp(new PackingService(dbService)), + new ReservationsMcp(new ReservationsService(dbService)), new DayNotesMcp(new DayNotesService(dbService)), new AssignmentsMcp(new AssignmentsService(dbService)), new CollabMcp(new CollabService(dbService)), diff --git a/server/tests/unit/mcp/resources.test.ts b/server/tests/unit/mcp/resources.test.ts index 924d29cdf..6782f9db4 100644 --- a/server/tests/unit/mcp/resources.test.ts +++ b/server/tests/unit/mcp/resources.test.ts @@ -244,32 +244,8 @@ describe('Resource: trek://trips/{tripId}/budget', () => { // The trek://trips/{tripId}/packing and .../packing/bags resources moved to the // DI-discovered PackingMcp — see tools-packing.test.ts. -describe('Resource: trek://trips/{tripId}/reservations', () => { - it('returns reservations for a trip', async () => { - const { user } = createUser(testDb); - const trip = createTrip(testDb, user.id); - createReservation(testDb, trip.id, { title: 'Flight to Paris', type: 'flight' }); - - await withHarness(user.id, async (harness) => { - const result = await harness.client.readResource({ uri: `trek://trips/${trip.id}/reservations` }); - const items = parseResourceResult(result) as any[]; - expect(items).toHaveLength(1); - expect(items[0].title).toBe('Flight to Paris'); - }); - }); - - it('returns access denied for unauthorized trip', async () => { - const { user } = createUser(testDb); - const { user: other } = createUser(testDb); - const trip = createTrip(testDb, other.id); - - await withHarness(user.id, async (harness) => { - const result = await harness.client.readResource({ uri: `trek://trips/${trip.id}/reservations` }); - const data = parseResourceResult(result) as any; - expect(data.error).toBeTruthy(); - }); - }); -}); +// The trek://trips/{tripId}/reservations resource moved to the DI-discovered +// ReservationsMcp — see tools-reservations.test.ts. // The trek://trips/{tripId}/days/{dayId}/notes resource moved to the // DI-discovered DayNotesMcp — see tools-notes.test.ts. diff --git a/server/tests/unit/mcp/tools-reservations.test.ts b/server/tests/unit/mcp/tools-reservations.test.ts index 6c2262b1c..bcbe1b7fd 100644 --- a/server/tests/unit/mcp/tools-reservations.test.ts +++ b/server/tests/unit/mcp/tools-reservations.test.ts @@ -37,7 +37,7 @@ import { createTables } from '../../../src/db/schema'; import { runMigrations } from '../../../src/db/migrations'; import { resetTestDb } from '../../helpers/test-db'; import { createUser, createTrip, createDay, createPlace, createReservation, createDayAssignment } from '../../helpers/factories'; -import { createMcpHarness, parseToolResult, type McpHarness } from '../../helpers/mcp-harness'; +import { createMcpHarness, parseToolResult, parseResourceResult, type McpHarness } from '../../helpers/mcp-harness'; beforeAll(() => { createTables(testDb); @@ -450,3 +450,35 @@ describe('Tool: link_hotel_accommodation', () => { }); }); }); + +// --------------------------------------------------------------------------- +// trek://trips/{tripId}/reservations resource (moved from the legacy +// registerResources into the DI-discovered ReservationsMcp) +// --------------------------------------------------------------------------- + +describe('Resource: trek://trips/{tripId}/reservations', () => { + it('returns reservations for a trip', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + createReservation(testDb, trip.id, { title: 'Flight to Paris', type: 'flight' }); + + await withHarness(user.id, async (h) => { + const result = await h.client.readResource({ uri: `trek://trips/${trip.id}/reservations` }); + const items = parseResourceResult(result) as { title: string }[]; + expect(items).toHaveLength(1); + expect(items[0].title).toBe('Flight to Paris'); + }); + }); + + it('returns access denied for unauthorized trip', async () => { + const { user } = createUser(testDb); + const { user: other } = createUser(testDb); + const trip = createTrip(testDb, other.id); + + await withHarness(user.id, async (h) => { + const result = await h.client.readResource({ uri: `trek://trips/${trip.id}/reservations` }); + const data = parseResourceResult(result) as { error?: string }; + expect(data.error).toBeTruthy(); + }); + }); +}); diff --git a/server/tests/unit/mcp/tools-transit.test.ts b/server/tests/unit/mcp/tools-transit.test.ts index ca2d5ee7c..7ce8c7700 100644 --- a/server/tests/unit/mcp/tools-transit.test.ts +++ b/server/tests/unit/mcp/tools-transit.test.ts @@ -42,8 +42,8 @@ vi.mock('../../../src/services/transitService', async (importOriginal) => ({ plan: planMock, })); vi.mock('../../../src/websocket', () => ({ broadcast: broadcastMock })); -vi.mock('../../../src/services/reservationService', async (importOriginal) => ({ - ...(await importOriginal()), +vi.mock('../../../src/nest/reservations/reservations.bridge', async (importOriginal) => ({ + ...(await importOriginal()), notifyBookingChange: notifyBookingChangeMock, })); vi.mock('../../../src/config', () => ({ 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 9b69692ba..879a5dd66 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 @@ -9,7 +9,6 @@ import { DatabaseService } from '../../../../src/nest/database/database.service' vi.mock('../../../../src/websocket', () => ({ broadcast: vi.fn() })); vi.mock('../../../../src/services/permissions', () => ({ checkPermission: vi.fn(() => true) })); vi.mock('../../../../src/services/tripAccess', () => ({ verifyTripAccess: vi.fn() })); -vi.mock('../../../../src/services/reservationService', () => ({ createReservation: vi.fn() })); vi.mock('../../../../src/services/placeService', () => ({ createPlace: vi.fn() })); vi.mock('../../../../src/services/mapsService', () => ({ searchNominatim: vi.fn() })); @@ -21,7 +20,8 @@ const file = (name = 'a.pdf') => ({ buffer: Buffer.from('x'), originalname: name function make(opts: { kit?: boolean; ai?: boolean; extract?: any; parse?: any }) { const extractor = { isAvailable: () => opts.kit ?? false, extract: vi.fn(opts.extract ?? (async () => [])) }; const llmParse = { isAvailable: () => opts.ai ?? false, parse: vi.fn(opts.parse ?? (async () => ({ kiItems: [], warnings: [] }))) }; - return { svc: new BookingImportService(extractor as any, llmParse as any, new DatabaseService(dbConn)), extractor, llmParse }; + const reservations = { create: vi.fn() }; + return { svc: new BookingImportService(extractor as any, llmParse as any, new DatabaseService(dbConn), reservations as never), extractor, llmParse, reservations }; } beforeEach(() => vi.clearAllMocks()); diff --git a/server/tests/unit/nest/reservations.controller.test.ts b/server/tests/unit/nest/reservations.controller.test.ts index a4190bca1..93d2e63cb 100644 --- a/server/tests/unit/nest/reservations.controller.test.ts +++ b/server/tests/unit/nest/reservations.controller.test.ts @@ -57,7 +57,7 @@ describe('ReservationsController (parity with the legacy /api/trips/:tripId/rese expect(broadcast).toHaveBeenCalledWith('5', 'accommodation:created', {}, 'sock'); expect(syncBudgetOnCreate).toHaveBeenCalledWith('5', 9, 'Hotel', 'lodging', { total_price: 200 }, 'sock'); expect(broadcast).toHaveBeenCalledWith('5', 'reservation:created', { reservation: { id: 9 } }, 'sock'); - expect(notifyBookingChange).toHaveBeenCalledWith('5', user, 'Hotel', 'lodging'); + expect(notifyBookingChange).toHaveBeenCalledWith('5', user.id, 'Hotel', 'lodging'); }); }); @@ -89,7 +89,7 @@ describe('ReservationsController (parity with the legacy /api/trips/:tripId/rese new ReservationsController(svc).update(user, '5', '9', { create_budget_entry: { total_price: 50 } }, 'sock'); expect(broadcast).toHaveBeenCalledWith('5', 'accommodation:updated', {}, 'sock'); expect(syncBudgetOnUpdate).toHaveBeenCalledWith('5', '9', '', undefined, 'Old', 'lodging', { total_price: 50 }, 'sock'); - expect(notifyBookingChange).toHaveBeenCalledWith('5', user, 'Old', 'lodging'); + expect(notifyBookingChange).toHaveBeenCalledWith('5', user.id, 'Old', 'lodging'); }); }); diff --git a/server/tests/unit/nest/reservations.service.test.ts b/server/tests/unit/nest/reservations.service.test.ts index 288aa0198..9a295722a 100644 --- a/server/tests/unit/nest/reservations.service.test.ts +++ b/server/tests/unit/nest/reservations.service.test.ts @@ -1,13 +1,35 @@ -import { describe, it, expect, vi, beforeEach } from 'vitest'; +/** + * Unit tests for the DI-native ReservationsService — RESV-SVC-001+ (real-SQL + * cases over the folded legacy services/reservationService.ts statements), + * RES-TRAV-001..005 (moved 1:1 from tests/unit/services/reservationTravelers.test.ts) + * and RESV-BRIDGE-001..009 (one delegation case per reservations.bridge export). + * Uses a real in-memory SQLite DB so the SQL logic — falsy-coercion defaults, + * COALESCE update semantics, day derivation, the accommodation/budget cascades + * and the TEXT accommodation_id normalization — is exercised faithfully. + */ +import { describe, it, expect, vi, beforeAll, beforeEach, afterAll } from 'vitest'; -// Mock the data + side-effect dependencies the service reaches into directly. -const { dbMock } = vi.hoisted(() => { - const stmt = { get: vi.fn(), all: vi.fn(() => []), run: vi.fn() }; - return { dbMock: { prepare: vi.fn(() => stmt), _stmt: stmt } }; +const { testDb, dbMock } = vi.hoisted(() => { + // eslint-disable-next-line @typescript-eslint/no-require-imports + const Database = require('better-sqlite3'); + const db = new Database(':memory:'); + db.exec('PRAGMA journal_mode = WAL'); + db.exec('PRAGMA foreign_keys = ON'); + db.exec('PRAGMA busy_timeout = 5000'); + const mock = { + db, closeDb: () => {}, reinitialize: () => {}, getPlaceWithTags: () => null, + canAccessTrip: (tripId: unknown, userId: number) => db.prepare(` + SELECT t.* FROM trips t + LEFT JOIN trip_members m ON m.trip_id = t.id AND m.user_id = ? + WHERE t.id = ? AND (t.user_id = ? OR m.user_id IS NOT NULL) + `).get(userId, tripId, userId), + isOwner: (tripId: unknown, userId: number) => + !!db.prepare('SELECT id FROM trips WHERE id = ? AND user_id = ?').get(tripId, userId), + }; + return { testDb: db, dbMock: mock }; }); -vi.mock('../../../src/db/database', () => ({ db: dbMock, closeDb: () => {}, reinitialize: () => {} })); -import { db as dbConn } from '../../../src/db/database'; -import { DatabaseService } from '../../../src/nest/database/database.service'; + +vi.mock('../../../src/db/database', () => dbMock); const { broadcast } = vi.hoisted(() => ({ broadcast: vi.fn() })); vi.mock('../../../src/websocket', () => ({ broadcast })); @@ -20,124 +42,695 @@ const { budget } = vi.hoisted(() => ({ })); vi.mock('../../../src/services/budgetService', () => budget); -const { resv } = vi.hoisted(() => ({ - resv: { - verifyTripAccess: vi.fn(), listReservations: vi.fn(), createReservation: vi.fn(), updatePositions: vi.fn(), - getReservation: vi.fn(), updateReservation: vi.fn(), deleteReservation: vi.fn(), - notifyBookingChange: vi.fn(), - setReservationTravelers: vi.fn(), loadTravelers: vi.fn(), getReservationWithJoins: vi.fn(), - }, -})); -vi.mock('../../../src/services/reservationService', () => resv); +const { notif } = vi.hoisted(() => ({ notif: { send: vi.fn().mockResolvedValue(undefined) } })); +vi.mock('../../../src/services/notificationService', () => notif); +import { createTables } from '../../../src/db/schema'; +import { runMigrations } from '../../../src/db/migrations'; +import { resetTestDb } from '../../helpers/test-db'; +import { createUser, createTrip, createReservation, createBudgetItem, createPlace, createDay, createDayAccommodation, addTripMember } from '../../helpers/factories'; +import { DatabaseService } from '../../../src/nest/database/database.service'; import { ReservationsService } from '../../../src/nest/reservations/reservations.service'; +import * as bridge from '../../../src/nest/reservations/reservations.bridge'; -function svc() { - return new ReservationsService(new DatabaseService(dbConn)); -} +const svc = new ReservationsService(new DatabaseService(testDb)); +beforeAll(() => { createTables(testDb); runMigrations(testDb); }); beforeEach(() => { + resetTestDb(testDb); vi.clearAllMocks(); vi.spyOn(console, 'error').mockImplementation(() => {}); }); +afterAll(() => testDb.close()); -describe('ReservationsService', () => { - it('canEdit delegates to checkPermission with reservation_edit', () => { - svc().canEdit({ user_id: 2 } as never, { id: 1, role: 'user' } as never); +/** Owner + trip helper for the common single-user cases. */ +function ownerTrip(overrides: Parameters[2] = {}) { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id, overrides); + return { user, trip }; +} + +describe('ReservationsService (DI-native, real SQL)', () => { + it('RESV-SVC-001: canEdit delegates to checkPermission with reservation_edit', () => { + svc.canEdit({ user_id: 2 } as never, { id: 1, role: 'user' } as never); expect(checkPermission).toHaveBeenCalledWith('reservation_edit', 'user', 2, 1, true); }); - it('list/create/getReservation/remove delegate to the legacy service', () => { - resv.listReservations.mockReturnValue([{ id: 1 }]); - expect(svc().list('5')).toEqual([{ id: 1 }]); - svc().create('5', { title: 'X' } as never); - expect(resv.createReservation).toHaveBeenCalledWith('5', { title: 'X' }); - svc().getReservation('9', '5'); - expect(resv.getReservation).toHaveBeenCalledWith('9', '5'); - svc().remove('9', '5'); - expect(resv.deleteReservation).toHaveBeenCalledWith('9', '5'); + describe('create', () => { + it('RESV-SVC-002: applies the legacy falsy-coercion defaults and re-selects the joined row', () => { + const { trip } = ownerTrip(); + const { reservation, accommodationCreated } = svc.create(String(trip.id), { title: 'Dinner', location: '' } as never); + expect(accommodationCreated).toBe(false); + expect(reservation).toMatchObject({ + title: 'Dinner', + type: 'other', // type || 'other' + status: 'pending', // status || 'pending' + location: null, // '' || null + accommodation_id: null, + endpoints: [], + travelers: [], + }); + }); + + it('RESV-SVC-003: derives day_id from reservation_time for non-hotel bookings', () => { + const { trip } = ownerTrip({ start_date: '2030-05-01', end_date: '2030-05-03' }); + const day2 = testDb.prepare('SELECT id FROM days WHERE trip_id = ? AND date = ?').get(trip.id, '2030-05-02') as { id: number }; + const { reservation } = svc.create(String(trip.id), { title: 'Tour', type: 'tour', reservation_time: '2030-05-02T10:00:00' }); + expect(reservation.day_id).toBe(day2.id); + // Out-of-range time clamps to the nearest day (create path clamps). + const day3 = testDb.prepare('SELECT id FROM days WHERE trip_id = ? AND date = ?').get(trip.id, '2030-05-03') as { id: number }; + const { reservation: clamped } = svc.create(String(trip.id), { title: 'Late', type: 'tour', reservation_time: '2030-06-20T10:00:00' }); + expect(clamped.day_id).toBe(day3.id); + }); + + it('RESV-SVC-004: auto-creates the accommodation for a hotel with create_accommodation', () => { + const { trip } = ownerTrip({ start_date: '2030-05-01', end_date: '2030-05-03' }); + const place = createPlace(testDb, trip.id); + const days = testDb.prepare('SELECT id FROM days WHERE trip_id = ? ORDER BY day_number').all(trip.id) as { id: number }[]; + const { reservation, accommodationCreated } = svc.create(String(trip.id), { + title: 'Hotel', type: 'hotel', + create_accommodation: { place_id: place.id, start_day_id: days[0].id, end_day_id: days[2].id, check_in: '15:00' }, + confirmation_number: 'ABC123', + }); + expect(accommodationCreated).toBe(true); + const acc = testDb.prepare('SELECT * FROM day_accommodations WHERE trip_id = ?').get(trip.id) as Record; + expect(acc).toMatchObject({ place_id: place.id, check_in: '15:00', confirmation: 'ABC123' }); + // TEXT accommodation_id column reads back normalized to an int. + expect(reservation.accommodation_id).toBe(Number(acc.id)); + expect(reservation.accommodation_name).toBe(place.name); + }); + + it('RESV-SVC-005: saves endpoints, skipping null-coordinate rows; sequence defaults to the post-filter index', () => { + const { trip } = ownerTrip(); + const endpoints = [ + { role: 'from', name: 'A', code: 'AAA', lat: 1, lng: 2, timezone: null, local_time: null, local_date: null }, + { role: 'to', name: 'NoGeo', code: null, lat: null, lng: null, timezone: null, local_time: null, local_date: null }, + { role: 'to', name: 'B', code: 'BBB', lat: 3, lng: 4, timezone: null, local_time: null, local_date: null }, + ]; + const { reservation } = svc.create(String(trip.id), { title: 'Bus', type: 'other', endpoints } as never); + const rows = testDb.prepare('SELECT name, sequence FROM reservation_endpoints WHERE reservation_id = ? ORDER BY sequence').all(reservation.id) as { name: string; sequence: number }[]; + // The index fallback runs AFTER the null-coord filter, so 'B' (third on + // the wire, second surviving row) gets sequence 1. + expect(rows).toEqual([{ name: 'A', sequence: 0 }, { name: 'B', sequence: 1 }]); + }); + + it('RESV-SVC-006 (quirk preserved): metadata check-in sync is gated on the raw accommodation_id, so an auto-created accommodation is not updated', () => { + const { trip } = ownerTrip({ start_date: '2030-05-01', end_date: '2030-05-02' }); + const place = createPlace(testDb, trip.id); + const days = testDb.prepare('SELECT id FROM days WHERE trip_id = ? ORDER BY day_number').all(trip.id) as { id: number }[]; + svc.create(String(trip.id), { + title: 'Hotel', type: 'hotel', + create_accommodation: { place_id: place.id, start_day_id: days[0].id, end_day_id: days[1].id }, + metadata: { check_in_time: '16:00' }, + }); + const acc = testDb.prepare('SELECT check_in FROM day_accommodations WHERE trip_id = ?').get(trip.id) as { check_in: string | null }; + expect(acc.check_in).toBeNull(); // the L416 gate reads accommodation_id, not resolvedAccommodationId + }); }); - describe('setTravelers', () => { + describe('update', () => { + it('RESV-SVC-007: an empty-string title silently keeps the old value (COALESCE + `|| null`)', () => { + const { trip } = ownerTrip(); + const res = createReservation(testDb, trip.id, { title: 'Old title' }); + const current = svc.getReservation(String(res.id), String(trip.id))!; + const { reservation } = svc.update(String(res.id), String(trip.id), { title: '', notes: 'updated' }, current); + expect(reservation.title).toBe('Old title'); + expect(reservation.notes).toBe('updated'); + }); + + it('RESV-SVC-008: switching to hotel forces reservation_time/reservation_end_time to null', () => { + const { trip } = ownerTrip(); + const res = createReservation(testDb, trip.id, { title: 'X', type: 'tour' }); + testDb.prepare('UPDATE reservations SET reservation_time = ? WHERE id = ?').run('2030-05-01T10:00:00', res.id); + const current = svc.getReservation(String(res.id), String(trip.id))!; + const { reservation } = svc.update(String(res.id), String(trip.id), { type: 'hotel' }, current); + expect(reservation.reservation_time).toBeNull(); + expect(reservation.reservation_end_time).toBeNull(); + }); + + it('RESV-SVC-009: a stale accommodation_id pointing at a deleted row is nulled out', () => { + const { trip } = ownerTrip(); + const res = createReservation(testDb, trip.id, { title: 'Hotel', type: 'hotel' }); + testDb.prepare('UPDATE reservations SET accommodation_id = ? WHERE id = ?').run('999', res.id); + const current = svc.getReservation(String(res.id), String(trip.id))!; + const { reservation } = svc.update(String(res.id), String(trip.id), { notes: 'n' }, current); + expect(reservation.accommodation_id).toBeNull(); + }); + + it('RESV-SVC-010: endpoints [] wipes stored endpoints; an absent field leaves them alone', () => { + const { trip } = ownerTrip(); + const res = createReservation(testDb, trip.id, { title: 'Bus' }); + testDb.prepare('INSERT INTO reservation_endpoints (reservation_id, role, sequence, name, lat, lng) VALUES (?, ?, 0, ?, 1, 2)').run(res.id, 'from', 'A'); + let current = svc.getReservation(String(res.id), String(trip.id))!; + svc.update(String(res.id), String(trip.id), { notes: 'no endpoints field' }, current); + expect(testDb.prepare('SELECT COUNT(*) as c FROM reservation_endpoints WHERE reservation_id = ?').get(res.id)).toEqual({ c: 1 }); + current = svc.getReservation(String(res.id), String(trip.id))!; + svc.update(String(res.id), String(trip.id), { endpoints: [] }, current); + expect(testDb.prepare('SELECT COUNT(*) as c FROM reservation_endpoints WHERE reservation_id = ?').get(res.id)).toEqual({ c: 0 }); + }); + }); + + describe('remove', () => { + it('RESV-SVC-011: cascades to the linked accommodation and budget item, reporting both', () => { + const { trip } = ownerTrip({ start_date: '2030-05-01', end_date: '2030-05-02' }); + const place = createPlace(testDb, trip.id); + const days = testDb.prepare('SELECT id FROM days WHERE trip_id = ? ORDER BY day_number').all(trip.id) as { id: number }[]; + const acc = createDayAccommodation(testDb, trip.id, place.id, days[0].id, days[1].id); + const res = createReservation(testDb, trip.id, { title: 'Hotel', type: 'hotel' }); + testDb.prepare('UPDATE reservations SET accommodation_id = ? WHERE id = ?').run(String(acc.id), res.id); + const item = createBudgetItem(testDb, trip.id); + testDb.prepare('UPDATE budget_items SET reservation_id = ? WHERE id = ?').run(res.id, item.id); + + const result = svc.remove(String(res.id), String(trip.id)); + expect(result.deleted).toMatchObject({ id: res.id, title: 'Hotel', type: 'hotel' }); + expect(result.accommodationDeleted).toBe(true); + expect(result.deletedBudgetItemId).toBe(item.id); + expect(testDb.prepare('SELECT COUNT(*) as c FROM reservations WHERE id = ?').get(res.id)).toEqual({ c: 0 }); + expect(testDb.prepare('SELECT COUNT(*) as c FROM day_accommodations WHERE id = ?').get(acc.id)).toEqual({ c: 0 }); + expect(testDb.prepare('SELECT COUNT(*) as c FROM budget_items WHERE id = ?').get(item.id)).toEqual({ c: 0 }); + }); + + it('RESV-SVC-012: returns the empty shape when the reservation is missing', () => { + const { trip } = ownerTrip(); + expect(svc.remove('999', String(trip.id))).toEqual({ deleted: undefined, accommodationDeleted: false, deletedBudgetItemId: null }); + }); + }); + + describe('updatePositions', () => { + it('RESV-SVC-013: without dayId updates the global day_plan_position (legacy branch)', () => { + const { trip } = ownerTrip(); + const r1 = createReservation(testDb, trip.id, { title: 'A' }); + const r2 = createReservation(testDb, trip.id, { title: 'B' }); + svc.updatePositions(String(trip.id), [{ id: r2.id, day_plan_position: 0 }, { id: r1.id, day_plan_position: 1 }]); + expect(testDb.prepare('SELECT day_plan_position FROM reservations WHERE id = ?').get(r2.id)).toEqual({ day_plan_position: 0 }); + expect(testDb.prepare('SELECT day_plan_position FROM reservations WHERE id = ?').get(r1.id)).toEqual({ day_plan_position: 1 }); + }); + + it('RESV-SVC-014: with dayId upserts per-day positions into reservation_day_positions', () => { + const { trip } = ownerTrip(); + const day = createDay(testDb, trip.id, { date: '2030-05-01' }); + const r1 = createReservation(testDb, trip.id, { title: 'A' }); + svc.updatePositions(String(trip.id), [{ id: r1.id, day_plan_position: 3 }], day.id); + svc.updatePositions(String(trip.id), [{ id: r1.id, day_plan_position: 5 }], day.id); // OR REPLACE + expect(testDb.prepare('SELECT position FROM reservation_day_positions WHERE reservation_id = ? AND day_id = ?').get(r1.id, day.id)).toEqual({ position: 5 }); + expect(testDb.prepare('SELECT day_plan_position FROM reservations WHERE id = ?').get(r1.id)).toEqual({ day_plan_position: null }); + }); + }); + + describe('list / getReservationWithJoins', () => { + it('RESV-SVC-015: attaches day_positions, endpoints, travelers and normalizes the TEXT accommodation_id', () => { + const { user, trip } = ownerTrip({ start_date: '2030-05-01', end_date: '2030-05-02' }); + const place = createPlace(testDb, trip.id); + const days = testDb.prepare('SELECT id FROM days WHERE trip_id = ? ORDER BY day_number').all(trip.id) as { id: number }[]; + const acc = createDayAccommodation(testDb, trip.id, place.id, days[0].id, days[1].id); + const res = createReservation(testDb, trip.id, { title: 'Hotel', type: 'hotel' }); + // The TEXT column quirk: the FK reads back as a numeric string like "14.0". + testDb.prepare('UPDATE reservations SET accommodation_id = ? WHERE id = ?').run(`${acc.id}.0`, res.id); + testDb.prepare('INSERT INTO reservation_day_positions (reservation_id, day_id, position) VALUES (?, ?, ?)').run(res.id, days[0].id, 2); + svc.setReservationTravelers(res.id, trip.id, [user.id]); + + const [row] = svc.list(String(trip.id)); + expect(row.accommodation_id).toBe(acc.id); + expect(row.accommodation_name).toBe(place.name); + expect(row.day_positions).toEqual({ [days[0].id]: 2 }); + expect(row.travelers).toHaveLength(1); + + const single = svc.getReservationWithJoins(res.id)!; + expect(single.accommodation_id).toBe(acc.id); + expect(single.travelers).toHaveLength(1); + }); + + it('RESV-SVC-016: getReservationWithJoins returns undefined for a missing id', () => { + expect(svc.getReservationWithJoins(999)).toBeUndefined(); + }); + }); + + describe('listUpcoming', () => { + it('RESV-SVC-017: returns future reservations across owned + member trips, skipping cancelled', () => { + const { user, trip } = ownerTrip(); + const future = createReservation(testDb, trip.id, { title: 'Future' }); + testDb.prepare('UPDATE reservations SET reservation_time = ? WHERE id = ?').run('2999-01-01T10:00:00', future.id); + const past = createReservation(testDb, trip.id, { title: 'Past' }); + testDb.prepare('UPDATE reservations SET reservation_time = ? WHERE id = ?').run('2000-01-01T10:00:00', past.id); + const cancelled = createReservation(testDb, trip.id, { title: 'Cancelled' }); + testDb.prepare("UPDATE reservations SET reservation_time = ?, status = 'cancelled' WHERE id = ?").run('2999-01-02T10:00:00', cancelled.id); + + const rows = svc.listUpcoming(user.id) as { title: string }[]; + expect(rows.map(r => r.title)).toEqual(['Future']); + }); + }); + + describe('resyncReservationDays', () => { + it('RESV-SVC-018: re-anchors a dated booking to the day matching its time; out-of-range stays untouched', () => { + const { trip } = ownerTrip({ start_date: '2030-05-01', end_date: '2030-05-03' }); + const days = testDb.prepare('SELECT id, date FROM days WHERE trip_id = ? ORDER BY day_number').all(trip.id) as { id: number; date: string }[]; + const res = createReservation(testDb, trip.id, { title: 'Tour', type: 'tour', day_id: days[0].id }); + testDb.prepare('UPDATE reservations SET reservation_time = ? WHERE id = ?').run('2030-05-03T09:00:00', res.id); + const outside = createReservation(testDb, trip.id, { title: 'Outside', type: 'tour', day_id: days[0].id }); + testDb.prepare('UPDATE reservations SET reservation_time = ? WHERE id = ?').run('2031-01-01T09:00:00', outside.id); + + svc.resyncReservationDays(String(trip.id)); + expect(testDb.prepare('SELECT day_id FROM reservations WHERE id = ?').get(res.id)).toEqual({ day_id: days[2].id }); + expect(testDb.prepare('SELECT day_id FROM reservations WHERE id = ?').get(outside.id)).toEqual({ day_id: days[0].id }); + }); + }); + + describe('setTravelers (controller-facing composition)', () => { it('returns null when the reservation is not on the trip (off-trip guard)', () => { - resv.getReservation.mockReturnValue(undefined); - expect(svc().setTravelers('9', '5', [2])).toBeNull(); - expect(resv.setReservationTravelers).not.toHaveBeenCalled(); + const { trip } = ownerTrip(); + expect(svc.setTravelers('999', String(trip.id), [2])).toBeNull(); }); it('assigns travelers and returns the refreshed travelers + reservation', () => { - resv.getReservation.mockReturnValue({ id: 9 }); - const travelers = [{ user_id: 2, username: 'Sam', avatar: null, is_guest: 0 }]; - resv.loadTravelers.mockReturnValue(travelers); - resv.getReservationWithJoins.mockReturnValue({ id: 9, travelers }); - expect(svc().setTravelers('9', '5', [2])).toEqual({ travelers, reservation: { id: 9, travelers } }); - expect(resv.setReservationTravelers).toHaveBeenCalledWith('9', '5', [2]); - expect(resv.getReservationWithJoins).toHaveBeenCalledWith(9); + const { user, trip } = ownerTrip(); + const res = createReservation(testDb, trip.id); + const result = svc.setTravelers(String(res.id), String(trip.id), [user.id])!; + expect(result.travelers).toHaveLength(1); + expect(result.travelers[0]).toMatchObject({ user_id: user.id }); + expect(result.reservation).toMatchObject({ id: res.id }); }); }); describe('syncBudgetOnCreate', () => { it('does nothing without a positive price', () => { - svc().syncBudgetOnCreate('5', 9, 'Hotel', 'lodging', undefined, 'sock'); - svc().syncBudgetOnCreate('5', 9, 'Hotel', 'lodging', { total_price: 0 }, 'sock'); + svc.syncBudgetOnCreate('5', 9, 'Hotel', 'lodging', undefined, 'sock'); + svc.syncBudgetOnCreate('5', 9, 'Hotel', 'lodging', { total_price: 0 }, 'sock'); expect(budget.linkBudgetItemToReservation).not.toHaveBeenCalled(); }); it('links a budget item and broadcasts budget:created', () => { budget.linkBudgetItemToReservation.mockReturnValue({ id: 7 }); - svc().syncBudgetOnCreate('5', 9, 'Hotel', 'lodging', { total_price: 200, category: 'Lodging' }, 'sock'); + svc.syncBudgetOnCreate('5', 9, 'Hotel', 'lodging', { total_price: 200, category: 'Lodging' }, 'sock'); expect(budget.linkBudgetItemToReservation).toHaveBeenCalledWith('5', 9, { name: 'Hotel', category: 'Lodging', total_price: 200 }); expect(broadcast).toHaveBeenCalledWith('5', 'budget:created', { item: { id: 7 } }, 'sock'); }); it('falls back to type then "Other" for the category and swallows errors', () => { budget.linkBudgetItemToReservation.mockImplementation(() => { throw new Error('boom'); }); - expect(() => svc().syncBudgetOnCreate('5', 9, 'Hotel', undefined, { total_price: 50 }, 'sock')).not.toThrow(); + expect(() => svc.syncBudgetOnCreate('5', 9, 'Hotel', undefined, { total_price: 50 }, 'sock')).not.toThrow(); }); }); describe('syncBudgetOnUpdate', () => { + function linkedItem(overrides: Parameters[2] = {}) { + const { trip } = ownerTrip(); + const res = createReservation(testDb, trip.id); + const item = createBudgetItem(testDb, trip.id, overrides); + testDb.prepare('UPDATE budget_items SET reservation_id = ? WHERE id = ?').run(res.id, item.id); + return { trip, res, item }; + } + it('deletes the linked item when the price is explicitly cleared (total_price 0)', () => { - dbMock._stmt.get.mockReturnValueOnce({ id: 7 }); - svc().syncBudgetOnUpdate('5', '9', 'Hotel', 'lodging', 'Hotel', 'lodging', { total_price: 0 }, 'sock'); - expect(budget.deleteBudgetItem).toHaveBeenCalledWith(7, '5'); - expect(broadcast).toHaveBeenCalledWith('5', 'budget:deleted', { itemId: 7 }, 'sock'); + const { trip, res, item } = linkedItem(); + svc.syncBudgetOnUpdate(String(trip.id), String(res.id), 'Hotel', 'lodging', 'Hotel', 'lodging', { total_price: 0 }, 'sock'); + expect(budget.deleteBudgetItem).toHaveBeenCalledWith(item.id, String(trip.id)); + expect(broadcast).toHaveBeenCalledWith(String(trip.id), 'budget:deleted', { itemId: item.id }, 'sock'); }); it('leaves the linked item alone when no budget entry is on the payload (no wipe)', () => { - svc().syncBudgetOnUpdate('5', '9', 'Hotel', 'lodging', 'Hotel', 'lodging', undefined, 'sock'); + const { trip, res } = linkedItem(); + svc.syncBudgetOnUpdate(String(trip.id), String(res.id), 'Hotel', 'lodging', 'Hotel', 'lodging', undefined, 'sock'); expect(budget.deleteBudgetItem).not.toHaveBeenCalled(); expect(budget.updateBudgetItem).not.toHaveBeenCalled(); expect(budget.createBudgetItem).not.toHaveBeenCalled(); }); it('syncs the linked expense category when the booking type changes', () => { - dbMock._stmt.get.mockReturnValueOnce({ id: 7, category: 'other' }); - budget.updateBudgetItem.mockReturnValue({ id: 7, category: 'flights' }); - svc().syncBudgetOnUpdate('5', '9', 'X', 'flight', 'X', 'other', undefined, 'sock'); - expect(budget.updateBudgetItem).toHaveBeenCalledWith(7, '5', { category: 'flights' }); - expect(broadcast).toHaveBeenCalledWith('5', 'budget:updated', { item: { id: 7, category: 'flights' } }, 'sock'); + const { trip, res, item } = linkedItem({ category: 'other' }); + budget.updateBudgetItem.mockReturnValue({ id: item.id, category: 'flights' }); + svc.syncBudgetOnUpdate(String(trip.id), String(res.id), 'X', 'flight', 'X', 'other', undefined, 'sock'); + expect(budget.updateBudgetItem).toHaveBeenCalledWith(item.id, String(trip.id), { category: 'flights' }); + expect(broadcast).toHaveBeenCalledWith(String(trip.id), 'budget:updated', { item: { id: item.id, category: 'flights' } }, 'sock'); }); it('updates an existing linked item when a price is provided', () => { - dbMock._stmt.get.mockReturnValueOnce({ id: 7 }); // existing lookup - budget.updateBudgetItem.mockReturnValue({ id: 7 }); - svc().syncBudgetOnUpdate('5', '9', 'New', 'lodging', 'Old', 'lodging', { total_price: 80 }, 'sock'); - expect(budget.updateBudgetItem).toHaveBeenCalledWith(7, '5', { name: 'New', category: 'lodging', total_price: 80 }); - expect(broadcast).toHaveBeenCalledWith('5', 'budget:updated', { item: { id: 7 } }, 'sock'); + const { trip, res, item } = linkedItem(); + budget.updateBudgetItem.mockReturnValue({ id: item.id }); + svc.syncBudgetOnUpdate(String(trip.id), String(res.id), 'New', 'lodging', 'Old', 'lodging', { total_price: 80 }, 'sock'); + expect(budget.updateBudgetItem).toHaveBeenCalledWith(item.id, String(trip.id), { name: 'New', category: 'lodging', total_price: 80 }); + expect(broadcast).toHaveBeenCalledWith(String(trip.id), 'budget:updated', { item: { id: item.id } }, 'sock'); }); it('creates + links a new item when none exists, using the current title fallback', () => { - dbMock._stmt.get.mockReturnValue(undefined); // no existing - budget.createBudgetItem.mockReturnValue({ id: 9 }); - svc().syncBudgetOnUpdate('5', '9', '', undefined, 'Old title', 'flight', { total_price: 120 }, 'sock'); - expect(budget.createBudgetItem).toHaveBeenCalledWith('5', { name: 'Old title', category: 'flight', total_price: 120 }); - expect(dbMock._stmt.run).toHaveBeenCalled(); // UPDATE budget_items SET reservation_id - expect(broadcast).toHaveBeenCalledWith('5', 'budget:created', { item: { id: 9, reservation_id: 9 } }, 'sock'); + const { trip } = ownerTrip(); + const res = createReservation(testDb, trip.id); + const created = createBudgetItem(testDb, trip.id); // the row the mocked create "returns" + budget.createBudgetItem.mockReturnValue({ id: created.id }); + svc.syncBudgetOnUpdate(String(trip.id), String(res.id), '', undefined, 'Old title', 'flight', { total_price: 120 }, 'sock'); + expect(budget.createBudgetItem).toHaveBeenCalledWith(String(trip.id), { name: 'Old title', category: 'flight', total_price: 120 }); + // The service back-links the created item to the reservation itself. + expect(testDb.prepare('SELECT reservation_id FROM budget_items WHERE id = ?').get(created.id)).toEqual({ reservation_id: res.id }); + expect(broadcast).toHaveBeenCalledWith(String(trip.id), 'budget:created', { item: { id: created.id, reservation_id: res.id } }, 'sock'); }); }); - it('notifyBookingChange resolves without throwing (fire-and-forget)', () => { - expect(() => svc().notifyBookingChange('5', { id: 1, email: 'a@b.c' } as never, 'Hotel', 'lodging')).not.toThrow(); + describe('notifyBookingChange', () => { + it('sends the booking_change notification with the legacy payload (fire-and-forget)', async () => { + const { user, trip } = ownerTrip(); + expect(() => svc.notifyBookingChange(String(trip.id), user.id, 'Hotel', 'lodging')).not.toThrow(); + await vi.waitFor(() => expect(notif.send).toHaveBeenCalled()); + expect(notif.send).toHaveBeenCalledWith({ + event: 'booking_change', + actorId: user.id, + scope: 'trip', + targetId: trip.id, + params: { trip: trip.title, actor: user.email, booking: 'Hotel', type: 'lodging', tripId: String(trip.id) }, + }); + }); + + it('is a silent no-op when the actor does not exist', async () => { + const { trip } = ownerTrip(); + svc.notifyBookingChange(String(trip.id), 999, 'Hotel', ''); + await new Promise(r => setTimeout(r, 10)); + expect(notif.send).not.toHaveBeenCalled(); + }); + }); +}); + +describe('setReservationTravelers / loadTravelers (#1517)', () => { + /** Create a named guest user (no credentials) and add it to the trip. */ + function addGuest(tripId: number, displayName: string) { + const { user } = createUser(testDb); + testDb.prepare('UPDATE users SET is_guest = 1, display_name = ? WHERE id = ?').run(displayName, user.id); + addTripMember(testDb, tripId, user.id); + return user; + } + + it('RES-TRAV-001: assigns trip members and returns them via loadTravelers', () => { + const { user: owner } = createUser(testDb); + const { user: member } = createUser(testDb); + const trip = createTrip(testDb, owner.id); + addTripMember(testDb, trip.id, member.id); + const res = createReservation(testDb, trip.id); + + svc.setReservationTravelers(res.id, trip.id, [member.id]); + + const travelers = svc.loadTravelers(res.id); + expect(travelers).toHaveLength(1); + expect(travelers[0]).toMatchObject({ user_id: member.id, username: member.username, is_guest: 0 }); + }); + + it('RES-TRAV-002: silently drops a user id that is not on the trip (cross-trip guard)', () => { + const { user: owner } = createUser(testDb); + const { user: member } = createUser(testDb); + const trip = createTrip(testDb, owner.id); + addTripMember(testDb, trip.id, member.id); + const res = createReservation(testDb, trip.id); + + // An outsider who belongs to a different trip only. + const { user: outsider } = createUser(testDb); + const otherTrip = createTrip(testDb, outsider.id); + addTripMember(testDb, otherTrip.id, outsider.id); + + svc.setReservationTravelers(res.id, trip.id, [member.id, outsider.id]); + + const ids = svc.loadTravelers(res.id).map(t => t.user_id); + expect(ids).toEqual([member.id]); + expect(ids).not.toContain(outsider.id); + }); + + it('RES-TRAV-003: assigns a named guest (users.is_guest = 1) joined via trip_members', () => { + const { user: owner } = createUser(testDb); + const trip = createTrip(testDb, owner.id); + const guest = addGuest(trip.id, 'Grandma'); + const res = createReservation(testDb, trip.id); + + svc.setReservationTravelers(res.id, trip.id, [guest.id]); + + const travelers = svc.loadTravelers(res.id); + expect(travelers).toHaveLength(1); + expect(travelers[0]).toMatchObject({ user_id: guest.id, username: 'Grandma', is_guest: 1 }); + }); + + it('RES-TRAV-004: re-setting replaces the previous set', () => { + const { user: owner } = createUser(testDb); + const { user: a } = createUser(testDb); + const { user: b } = createUser(testDb); + const trip = createTrip(testDb, owner.id); + addTripMember(testDb, trip.id, a.id); + addTripMember(testDb, trip.id, b.id); + const res = createReservation(testDb, trip.id); + + svc.setReservationTravelers(res.id, trip.id, [a.id]); + expect(svc.loadTravelers(res.id).map(t => t.user_id)).toEqual([a.id]); + + svc.setReservationTravelers(res.id, trip.id, [b.id]); + const ids = svc.loadTravelers(res.id).map(t => t.user_id); + expect(ids).toEqual([b.id]); + expect(ids).not.toContain(a.id); + + // Clearing removes everyone. + svc.setReservationTravelers(res.id, trip.id, []); + expect(svc.loadTravelers(res.id)).toHaveLength(0); + }); + + it('RES-TRAV-005: list attaches travelers[] per reservation', () => { + const { user: owner } = createUser(testDb); + const { user: member } = createUser(testDb); + const trip = createTrip(testDb, owner.id); + addTripMember(testDb, trip.id, member.id); + const withTravelers = createReservation(testDb, trip.id, { title: 'Flight' }); + const withoutTravelers = createReservation(testDb, trip.id, { title: 'Hotel' }); + + svc.setReservationTravelers(withTravelers.id, trip.id, [member.id]); + + const list = svc.list(trip.id); + const assigned = list.find(r => r.id === withTravelers.id)!; + const empty = list.find(r => r.id === withoutTravelers.id)!; + + expect(assigned.travelers).toHaveLength(1); + expect(assigned.travelers![0]).toMatchObject({ user_id: member.id, username: member.username }); + expect(empty.travelers).toEqual([]); + }); +}); + +describe('reservations.bridge (legacy-named delegation for outside-container consumers)', () => { + it('RESV-BRIDGE-001: listReservations delegates to ReservationsService.list', () => { + const { trip } = ownerTrip(); + createReservation(testDb, trip.id, { title: 'A' }); + expect(bridge.listReservations(trip.id).map(r => r.title)).toEqual(['A']); + }); + + it('RESV-BRIDGE-002: loadEndpointsByTrip groups endpoints per reservation', () => { + const { trip } = ownerTrip(); + const res = createReservation(testDb, trip.id); + testDb.prepare('INSERT INTO reservation_endpoints (reservation_id, role, sequence, name, lat, lng) VALUES (?, ?, 0, ?, 1, 2)').run(res.id, 'from', 'A'); + const map = bridge.loadEndpointsByTrip(trip.id); + expect(map.get(res.id)).toHaveLength(1); + }); + + it('RESV-BRIDGE-003: resyncReservationDays re-anchors through the bridge', () => { + const { trip } = ownerTrip({ start_date: '2030-05-01', end_date: '2030-05-02' }); + const days = testDb.prepare('SELECT id FROM days WHERE trip_id = ? ORDER BY day_number').all(trip.id) as { id: number }[]; + const res = createReservation(testDb, trip.id, { title: 'T', type: 'tour', day_id: days[0].id }); + testDb.prepare('UPDATE reservations SET reservation_time = ? WHERE id = ?').run('2030-05-02T09:00:00', res.id); + bridge.resyncReservationDays(trip.id); + expect(testDb.prepare('SELECT day_id FROM reservations WHERE id = ?').get(res.id)).toEqual({ day_id: days[1].id }); + }); + + it('RESV-BRIDGE-004: createReservation creates through the bridge', () => { + const { trip } = ownerTrip(); + const { reservation } = bridge.createReservation(trip.id, { title: 'Bridge' }); + expect(reservation).toMatchObject({ title: 'Bridge', type: 'other' }); + }); + + it('RESV-BRIDGE-005: getReservation is trip-scoped', () => { + const { trip } = ownerTrip(); + const res = createReservation(testDb, trip.id); + expect(bridge.getReservation(res.id, trip.id)).toMatchObject({ id: res.id }); + expect(bridge.getReservation(res.id, trip.id + 1)).toBeUndefined(); + }); + + it('RESV-BRIDGE-006: getReservationWithJoins attaches endpoints + travelers', () => { + const { trip } = ownerTrip(); + const res = createReservation(testDb, trip.id); + expect(bridge.getReservationWithJoins(res.id)).toMatchObject({ id: res.id, endpoints: [], travelers: [] }); + }); + + it('RESV-BRIDGE-007: updateReservation updates through the bridge', () => { + const { trip } = ownerTrip(); + const res = createReservation(testDb, trip.id, { title: 'Old' }); + const current = bridge.getReservation(res.id, trip.id)!; + const { reservation } = bridge.updateReservation(res.id, trip.id, { title: 'New' }, current); + expect(reservation.title).toBe('New'); + }); + + it('RESV-BRIDGE-008: deleteReservation deletes through the bridge', () => { + const { trip } = ownerTrip(); + const res = createReservation(testDb, trip.id); + const { deleted } = bridge.deleteReservation(res.id, trip.id); + expect(deleted).toMatchObject({ id: res.id }); + expect(bridge.getReservation(res.id, trip.id)).toBeUndefined(); + }); + + it('RESV-BRIDGE-009: notifyBookingChange fires the notification through the bridge', async () => { + const { user, trip } = ownerTrip(); + bridge.notifyBookingChange(trip.id, user.id, 'Bridge booking', 'other'); + await vi.waitFor(() => expect(notif.send).toHaveBeenCalled()); + }); +}); + +describe('ReservationsService — legacy branch parity (coverage of the folded conditionals)', () => { + it('RESV-SVC-019: create with a linked accommodation_id syncs metadata times + confirmation onto it', () => { + const { trip } = ownerTrip({ start_date: '2030-05-01', end_date: '2030-05-02' }); + const place = createPlace(testDb, trip.id); + const days = testDb.prepare('SELECT id FROM days WHERE trip_id = ? ORDER BY day_number').all(trip.id) as { id: number }[]; + const acc = createDayAccommodation(testDb, trip.id, place.id, days[0].id, days[1].id); + svc.create(String(trip.id), { + title: 'Hotel', type: 'hotel', accommodation_id: acc.id, + metadata: { check_in_time: '15:00', check_out_time: '11:00' }, + confirmation_number: 'CN-1', + }); + const row = testDb.prepare('SELECT check_in, check_out, confirmation FROM day_accommodations WHERE id = ?').get(acc.id); + expect(row).toEqual({ check_in: '15:00', check_out: '11:00', confirmation: 'CN-1' }); + }); + + it('RESV-SVC-020: create with a malformed reservation_time derives no day', () => { + const { trip } = ownerTrip({ start_date: '2030-05-01', end_date: '2030-05-02' }); + const { reservation } = svc.create(String(trip.id), { title: 'X', type: 'tour', reservation_time: 'not-a-date' }); + expect(reservation.day_id).toBeNull(); + }); + + it('RESV-SVC-021: hotel update with create_accommodation updates the linked accommodation in place', () => { + const { trip } = ownerTrip({ start_date: '2030-05-01', end_date: '2030-05-03' }); + const place = createPlace(testDb, trip.id); + const place2 = createPlace(testDb, trip.id, { name: 'Other Hotel' }); + const days = testDb.prepare('SELECT id FROM days WHERE trip_id = ? ORDER BY day_number').all(trip.id) as { id: number }[]; + const acc = createDayAccommodation(testDb, trip.id, place.id, days[0].id, days[1].id); + const res = createReservation(testDb, trip.id, { title: 'Hotel', type: 'hotel' }); + testDb.prepare('UPDATE reservations SET accommodation_id = ? WHERE id = ?').run(String(acc.id), res.id); + const current = svc.getReservation(String(res.id), String(trip.id))!; + const { accommodationChanged } = svc.update(String(res.id), String(trip.id), { + type: 'hotel', + create_accommodation: { place_id: place2.id, start_day_id: days[1].id, end_day_id: days[2].id, check_in: '14:00' }, + confirmation_number: 'CN-2', + }, current); + expect(accommodationChanged).toBe(true); + const row = testDb.prepare('SELECT place_id, start_day_id, end_day_id, check_in, confirmation FROM day_accommodations WHERE id = ?').get(acc.id); + expect(row).toEqual({ place_id: place2.id, start_day_id: days[1].id, end_day_id: days[2].id, check_in: '14:00', confirmation: 'CN-2' }); + }); + + it('RESV-SVC-022: hotel update inserts a new accommodation when none is linked and a place is given', () => { + const { trip } = ownerTrip({ start_date: '2030-05-01', end_date: '2030-05-02' }); + const place = createPlace(testDb, trip.id); + const days = testDb.prepare('SELECT id FROM days WHERE trip_id = ? ORDER BY day_number').all(trip.id) as { id: number }[]; + const res = createReservation(testDb, trip.id, { title: 'Hotel', type: 'hotel' }); + const current = svc.getReservation(String(res.id), String(trip.id))!; + const { reservation, accommodationChanged } = svc.update(String(res.id), String(trip.id), { + type: 'hotel', + create_accommodation: { place_id: place.id, start_day_id: days[0].id, end_day_id: days[1].id }, + }, current); + expect(accommodationChanged).toBe(true); + expect(reservation.accommodation_id).not.toBeNull(); + expect(testDb.prepare('SELECT COUNT(*) as c FROM day_accommodations WHERE trip_id = ?').get(trip.id)).toEqual({ c: 1 }); + }); + + it('RESV-SVC-023: explicit update fields bind their values; empty strings null; absent fields keep current', () => { + const { trip } = ownerTrip(); + const place = createPlace(testDb, trip.id); + const res = createReservation(testDb, trip.id, { title: 'T', type: 'tour' }); + testDb.prepare("UPDATE reservations SET location = 'Loc', notes = 'N', url = 'U', confirmation_number = 'C' WHERE id = ?").run(res.id); + let current = svc.getReservation(String(res.id), String(trip.id))!; + let { reservation } = svc.update(String(res.id), String(trip.id), { + location: 'NewLoc', notes: '', url: 'https://x', confirmation_number: 'NEW', + status: 'confirmed', needs_review: true, metadata: { a: 1 }, place_id: place.id, + }, current); + expect(reservation).toMatchObject({ + location: 'NewLoc', notes: null, url: 'https://x', confirmation_number: 'NEW', + status: 'confirmed', needs_review: 1, metadata: JSON.stringify({ a: 1 }), place_id: place.id, + }); + current = svc.getReservation(String(res.id), String(trip.id))!; + ({ reservation } = svc.update(String(res.id), String(trip.id), { title: 'T2' }, current)); + expect(reservation).toMatchObject({ title: 'T2', location: 'NewLoc', url: 'https://x', place_id: place.id }); + }); + + it('RESV-SVC-024: explicit day_id / end_day_id updates win over derivation; explicit null end clears', () => { + const { trip } = ownerTrip({ start_date: '2030-05-01', end_date: '2030-05-03' }); + const days = testDb.prepare('SELECT id FROM days WHERE trip_id = ? ORDER BY day_number').all(trip.id) as { id: number }[]; + const res = createReservation(testDb, trip.id, { title: 'T', type: 'tour' }); + let current = svc.getReservation(String(res.id), String(trip.id))!; + let { reservation } = svc.update(String(res.id), String(trip.id), { day_id: days[1].id, end_day_id: days[2].id }, current); + expect(reservation).toMatchObject({ day_id: days[1].id, end_day_id: days[2].id }); + current = svc.getReservation(String(res.id), String(trip.id))!; + ({ reservation } = svc.update(String(res.id), String(trip.id), { end_day_id: null, reservation_end_time: '2030-05-02T18:00:00' }, current)); + // explicit end_day_id null wins over the end-time derivation + expect(reservation.end_day_id).toBeNull(); + current = svc.getReservation(String(res.id), String(trip.id))!; + ({ reservation } = svc.update(String(res.id), String(trip.id), { reservation_end_time: '2030-05-02T18:00:00' }, current)); + // no explicit end day + an end time -> derived + expect(reservation.end_day_id).toBe(days[1].id); + }); + + it('RESV-SVC-025: with metadata absent, the stored metadata JSON drives the accommodation sync (confirmation falls back to current)', () => { + const { trip } = ownerTrip({ start_date: '2030-05-01', end_date: '2030-05-02' }); + const place = createPlace(testDb, trip.id); + const days = testDb.prepare('SELECT id FROM days WHERE trip_id = ? ORDER BY day_number').all(trip.id) as { id: number }[]; + const acc = createDayAccommodation(testDb, trip.id, place.id, days[0].id, days[1].id); + const res = createReservation(testDb, trip.id, { title: 'Hotel', type: 'hotel' }); + testDb.prepare("UPDATE reservations SET accommodation_id = ?, metadata = ?, confirmation_number = 'KEEP' WHERE id = ?") + .run(String(acc.id), JSON.stringify({ check_in_time: '16:00' }), res.id); + const current = svc.getReservation(String(res.id), String(trip.id))!; + svc.update(String(res.id), String(trip.id), { notes: 'touch' }, current); + const row = testDb.prepare('SELECT check_in, confirmation FROM day_accommodations WHERE id = ?').get(acc.id); + expect(row).toEqual({ check_in: '16:00', confirmation: 'KEEP' }); + }); + + it('RESV-SVC-026: resync derives the end day too, keeping the stored one when the end time is out of range', () => { + const { trip } = ownerTrip({ start_date: '2030-05-01', end_date: '2030-05-03' }); + const days = testDb.prepare('SELECT id FROM days WHERE trip_id = ? ORDER BY day_number').all(trip.id) as { id: number }[]; + const res = createReservation(testDb, trip.id, { title: 'T', type: 'tour', day_id: days[0].id }); + testDb.prepare('UPDATE reservations SET reservation_time = ?, reservation_end_time = ?, end_day_id = ? WHERE id = ?') + .run('2030-05-02T09:00:00', '2031-01-01T09:00:00', days[0].id, res.id); + svc.resyncReservationDays(String(trip.id)); + // start re-anchored to day 2; out-of-range end time keeps the stored end day + expect(testDb.prepare('SELECT day_id, end_day_id FROM reservations WHERE id = ?').get(res.id)) + .toEqual({ day_id: days[1].id, end_day_id: days[0].id }); + // A booking already on the right day is left untouched (no-change skip). + svc.resyncReservationDays(String(trip.id)); + expect(testDb.prepare('SELECT day_id FROM reservations WHERE id = ?').get(res.id)).toEqual({ day_id: days[1].id }); + }); + + it('RESV-SVC-027: listUpcoming includes a timeless reservation whose day is in the future', () => { + const { user, trip } = ownerTrip(); + const day = createDay(testDb, trip.id, { date: '2999-06-01' }); + createReservation(testDb, trip.id, { title: 'Timeless', day_id: day.id }); + const rows = svc.listUpcoming(user.id) as { title: string }[]; + expect(rows.map(r => r.title)).toContain('Timeless'); + }); + + it('RESV-SVC-028: list merges multiple per-day positions for one reservation', () => { + const { trip } = ownerTrip(); + const d1 = createDay(testDb, trip.id, { date: '2030-05-01' }); + const d2 = createDay(testDb, trip.id, { date: '2030-05-02' }); + const res = createReservation(testDb, trip.id); + testDb.prepare('INSERT INTO reservation_day_positions (reservation_id, day_id, position) VALUES (?, ?, ?)').run(res.id, d1.id, 0); + testDb.prepare('INSERT INTO reservation_day_positions (reservation_id, day_id, position) VALUES (?, ?, ?)').run(res.id, d2.id, 3); + const [row] = svc.list(String(trip.id)); + expect(row.day_positions).toEqual({ [d1.id]: 0, [d2.id]: 3 }); + }); + + it('RESV-SVC-029: notifyBookingChange falls back to "Untitled" and type "booking"', async () => { + const { user } = createUser(testDb); + svc.notifyBookingChange('424242', user.id, 'B', ''); + await vi.waitFor(() => expect(notif.send).toHaveBeenCalled()); + expect(notif.send).toHaveBeenCalledWith(expect.objectContaining({ + params: expect.objectContaining({ trip: 'Untitled', type: 'booking' }), + })); + }); + + it('RESV-SVC-030: syncBudgetOnUpdate type-change sync is a no-op without a linked item or when the category was hand-picked', () => { + const { trip } = ownerTrip(); + const res = createReservation(testDb, trip.id); + // no linked item at all + svc.syncBudgetOnUpdate(String(trip.id), String(res.id), 'X', 'flight', 'X', 'other', undefined, undefined); + expect(budget.updateBudgetItem).not.toHaveBeenCalled(); + // linked item whose category no longer matches the auto-derived one + const item = createBudgetItem(testDb, trip.id, { category: 'Hand picked' }); + testDb.prepare('UPDATE budget_items SET reservation_id = ? WHERE id = ?').run(res.id, item.id); + svc.syncBudgetOnUpdate(String(trip.id), String(res.id), 'X', 'flight', 'X', 'other', undefined, undefined); + expect(budget.updateBudgetItem).not.toHaveBeenCalled(); + // explicit clear with no linked item deletes nothing + svc.syncBudgetOnUpdate(String(trip.id), '999999', 'X', 'flight', 'X', 'flight', { total_price: 0 }, undefined); + expect(budget.deleteBudgetItem).not.toHaveBeenCalled(); }); }); diff --git a/server/tests/unit/nest/trips.service.test.ts b/server/tests/unit/nest/trips.service.test.ts index ef705b720..490fa40b9 100644 --- a/server/tests/unit/nest/trips.service.test.ts +++ b/server/tests/unit/nest/trips.service.test.ts @@ -31,19 +31,19 @@ vi.mock('../../../src/services/budgetService', () => ({ listBudgetItems: () => [], rebaseTripCurrency: vi.fn(async () => {}), })); -vi.mock('../../../src/services/reservationService', () => ({ listReservations: () => [] })); - import { TripsService } from '../../../src/nest/trips/trips.service'; import type { TodoService } from '../../../src/nest/todo/todo.service'; import type { PackingService } from '../../../src/nest/packing/packing.service'; import type { FilesService } from '../../../src/nest/files/files.service'; +import type { ReservationsService } from '../../../src/nest/reservations/reservations.service'; import { rebaseTripCurrency } from '../../../src/services/budgetService'; const todoStub = { listItems: () => [] } as unknown as TodoService; const packingListItems = vi.fn(() => []); const packingStub = { listItems: packingListItems } as unknown as PackingService; const filesStub = { listFiles: () => [] } as unknown as FilesService; -function svc() { return new TripsService(new DatabaseService(dbConn), todoStub, packingStub, filesStub); } +const reservationsStub = { list: () => [] } as unknown as ReservationsService; +function svc() { return new TripsService(new DatabaseService(dbConn), todoStub, packingStub, filesStub, reservationsStub); } beforeEach(() => vi.clearAllMocks()); describe('TripsService (wrapper delegation + bundle/copy/notify helpers)', () => { 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 163b236a7..481af84b5 100644 --- a/server/tests/unit/plugins/plugin-host-deps.factory.test.ts +++ b/server/tests/unit/plugins/plugin-host-deps.factory.test.ts @@ -245,6 +245,7 @@ const reservationsStub = { list(tid: string) { return [{ id: 1, trip_id: Number(tid), title: 'Flight' }]; }, syncBudgetOnCreate() {}, syncBudgetOnUpdate() {}, + notifyBookingChange() {}, } as unknown as ReservationsService; vi.mock('../../../src/services/journeyService', () => ({ // Consumed by the real assignments.service.ts module (loaded un-mocked as a diff --git a/server/tests/unit/services/airtrailWriteGate.test.ts b/server/tests/unit/services/airtrailWriteGate.test.ts index 81964989a..5a2c56cdc 100644 --- a/server/tests/unit/services/airtrailWriteGate.test.ts +++ b/server/tests/unit/services/airtrailWriteGate.test.ts @@ -10,7 +10,7 @@ vi.mock('../../../src/db/database', () => ({ db: { prepare: vi.fn() } })); vi.mock('../../../src/services/adminService', () => ({ isAddonEnabled: vi.fn(() => true) })); vi.mock('../../../src/services/auditLog', () => ({ logError: vi.fn(), logInfo: vi.fn() })); vi.mock('../../../src/websocket', () => ({ broadcast: vi.fn() })); -vi.mock('../../../src/services/reservationService', () => ({ +vi.mock('../../../src/nest/reservations/reservations.bridge', () => ({ getReservation: vi.fn(), getReservationWithJoins: vi.fn(), updateReservation: vi.fn(), @@ -33,7 +33,7 @@ vi.mock('../../../src/services/airtrail/airtrailService', () => ({ import { pushReservationToAirtrail, runAirtrailSyncForUser } from '../../../src/services/airtrail/airtrailSync'; import { db } from '../../../src/db/database'; -import { getReservation, getReservationWithJoins, updateReservation } from '../../../src/services/reservationService'; +import { getReservation, getReservationWithJoins, updateReservation } from '../../../src/nest/reservations/reservations.bridge'; import { getFlight, listFlights, saveFlight } from '../../../src/services/airtrail/airtrailClient'; import { isAirtrailWriteEnabled, getAirtrailCredentials } from '../../../src/services/airtrail/airtrailService'; diff --git a/server/tests/unit/services/reservationTravelers.test.ts b/server/tests/unit/services/reservationTravelers.test.ts deleted file mode 100644 index abd1867b3..000000000 --- a/server/tests/unit/services/reservationTravelers.test.ts +++ /dev/null @@ -1,121 +0,0 @@ -/** - * Unit tests for reservation traveler assignment — RES-TRAV-001..005. - * The #1517 backend: assign trip members / named guests to a booking, with a - * cross-trip guard so a stale client can't leak a user from another trip. - */ -import { describe, it, expect, beforeAll, beforeEach, afterAll, vi } from 'vitest'; - -const { testDb, dbMock } = vi.hoisted(() => { - const Database = require('better-sqlite3'); - const db = new Database(':memory:'); - db.exec('PRAGMA foreign_keys = ON'); - return { testDb: db, dbMock: { db } }; -}); -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, createReservation, addTripMember } from '../../helpers/factories'; -import { setReservationTravelers, loadTravelers, listReservations } from '../../../src/services/reservationService'; - -beforeAll(() => { createTables(testDb); runMigrations(testDb); }); -beforeEach(() => resetTestDb(testDb)); -afterAll(() => testDb.close()); - -/** Create a named guest user (no credentials) and add it to the trip. */ -function addGuest(tripId: number, displayName: string) { - const { user } = createUser(testDb); - testDb.prepare('UPDATE users SET is_guest = 1, display_name = ? WHERE id = ?').run(displayName, user.id); - addTripMember(testDb, tripId, user.id); - return user; -} - -describe('setReservationTravelers / loadTravelers (#1517)', () => { - it('RES-TRAV-001: assigns trip members and returns them via loadTravelers', () => { - const { user: owner } = createUser(testDb); - const { user: member } = createUser(testDb); - const trip = createTrip(testDb, owner.id); - addTripMember(testDb, trip.id, member.id); - const res = createReservation(testDb, trip.id); - - setReservationTravelers(res.id, trip.id, [member.id]); - - const travelers = loadTravelers(res.id); - expect(travelers).toHaveLength(1); - expect(travelers[0]).toMatchObject({ user_id: member.id, username: member.username, is_guest: 0 }); - }); - - it('RES-TRAV-002: silently drops a user id that is not on the trip (cross-trip guard)', () => { - const { user: owner } = createUser(testDb); - const { user: member } = createUser(testDb); - const trip = createTrip(testDb, owner.id); - addTripMember(testDb, trip.id, member.id); - const res = createReservation(testDb, trip.id); - - // An outsider who belongs to a different trip only. - const { user: outsider } = createUser(testDb); - const otherTrip = createTrip(testDb, outsider.id); - addTripMember(testDb, otherTrip.id, outsider.id); - - setReservationTravelers(res.id, trip.id, [member.id, outsider.id]); - - const ids = loadTravelers(res.id).map(t => t.user_id); - expect(ids).toEqual([member.id]); - expect(ids).not.toContain(outsider.id); - }); - - it('RES-TRAV-003: assigns a named guest (users.is_guest = 1) joined via trip_members', () => { - const { user: owner } = createUser(testDb); - const trip = createTrip(testDb, owner.id); - const guest = addGuest(trip.id, 'Grandma'); - const res = createReservation(testDb, trip.id); - - setReservationTravelers(res.id, trip.id, [guest.id]); - - const travelers = loadTravelers(res.id); - expect(travelers).toHaveLength(1); - expect(travelers[0]).toMatchObject({ user_id: guest.id, username: 'Grandma', is_guest: 1 }); - }); - - it('RES-TRAV-004: re-setting replaces the previous set', () => { - const { user: owner } = createUser(testDb); - const { user: a } = createUser(testDb); - const { user: b } = createUser(testDb); - const trip = createTrip(testDb, owner.id); - addTripMember(testDb, trip.id, a.id); - addTripMember(testDb, trip.id, b.id); - const res = createReservation(testDb, trip.id); - - setReservationTravelers(res.id, trip.id, [a.id]); - expect(loadTravelers(res.id).map(t => t.user_id)).toEqual([a.id]); - - setReservationTravelers(res.id, trip.id, [b.id]); - const ids = loadTravelers(res.id).map(t => t.user_id); - expect(ids).toEqual([b.id]); - expect(ids).not.toContain(a.id); - - // Clearing removes everyone. - setReservationTravelers(res.id, trip.id, []); - expect(loadTravelers(res.id)).toHaveLength(0); - }); - - it('RES-TRAV-005: listReservations attaches travelers[] per reservation', () => { - const { user: owner } = createUser(testDb); - const { user: member } = createUser(testDb); - const trip = createTrip(testDb, owner.id); - addTripMember(testDb, trip.id, member.id); - const withTravelers = createReservation(testDb, trip.id, { title: 'Flight' }); - const withoutTravelers = createReservation(testDb, trip.id, { title: 'Hotel' }); - - setReservationTravelers(withTravelers.id, trip.id, [member.id]); - - const list = listReservations(trip.id); - const assigned = list.find(r => r.id === withTravelers.id); - const empty = list.find(r => r.id === withoutTravelers.id); - - expect(assigned.travelers).toHaveLength(1); - expect(assigned.travelers[0]).toMatchObject({ user_id: member.id, username: member.username }); - expect(empty.travelers).toEqual([]); - }); -});