diff --git a/server/src/db/migrations.ts b/server/src/db/migrations.ts index 60e267bb1..74af2d634 100644 --- a/server/src/db/migrations.ts +++ b/server/src/db/migrations.ts @@ -4731,6 +4731,17 @@ function runMigrations(db: Database.Database): void { const hasSource = db.prepare("SELECT 1 FROM pragma_table_info('places') WHERE name = 'source'").get(); if (!hasSource) db.exec('ALTER TABLE places ADD COLUMN source TEXT'); }, + + // Provenance for a day stop that a lodging booking put there rather than the + // traveller: it carries the stay's id, so moving or deleting the booking can + // move or delete exactly that stop and never one somebody placed by hand. + // Deliberately left NULL for every row that already exists: everything planned + // before this shipped was planned by hand, and a booking must not start + // claiming ownership of it. + () => { + const hasColumn = db.prepare("SELECT 1 FROM pragma_table_info('day_assignments') WHERE name = 'accommodation_id'").get(); + if (!hasColumn) db.exec('ALTER TABLE day_assignments ADD COLUMN accommodation_id INTEGER'); + }, ]; if (currentVersion < migrations.length) { diff --git a/server/src/nest/accommodations/accommodations.controller.ts b/server/src/nest/accommodations/accommodations.controller.ts index 8c7b32496..88505078a 100644 --- a/server/src/nest/accommodations/accommodations.controller.ts +++ b/server/src/nest/accommodations/accommodations.controller.ts @@ -37,6 +37,11 @@ type AccommodationBody = { * found) bodies, create 201 / rest 200, and the cascade broadcasts (a created * accommodation also emits reservation:created; a delete emits the linked * reservation/budget deletions) with the forwarded X-Socket-Id. + * + * A booking also writes the day stop that puts it on the route, so the answers + * carry that stop alongside the existing fields: the broadcast skips the socket + * that sent the request, and the client that booked the night has to be able to + * draw it without waiting for a reload. */ @Controller('api/trips/:tripId/accommodations') // TripAccessGuard resolves :tripId and 404s a trip the user cannot reach; mutations @@ -70,10 +75,14 @@ export class AccommodationsController { if (errors.length > 0) { throw new HttpException({ error: errors[0].message }, 404); } - const accommodation = this.accommodations.create(tripId, { place_id, start_day_id, end_day_id, check_in, check_in_end, check_out, confirmation, notes } as never); + const { accommodation, mirror } = this.accommodations.create(tripId, { place_id, start_day_id, end_day_id, check_in, check_in_end, check_out, confirmation, notes } as never); this.accommodations.broadcast(tripId, 'accommodation:created', { accommodation }, socketId); this.accommodations.broadcast(tripId, 'reservation:created', {}, socketId); - return { accommodation }; + this.accommodations.announceMirror(tripId, mirror, (event, payload) => this.accommodations.broadcast(tripId, event, payload, socketId), socketId); + // The stop rides in the answer as well: the broadcast above deliberately skips + // the socket that sent the request, so without it the very session that booked + // the night would be the one that cannot see it until a reload. + return { accommodation, assignment: mirror.created }; } @RequirePermission('day_edit') @@ -95,9 +104,10 @@ export class AccommodationsController { if (errors.length > 0) { throw new HttpException({ error: errors[0].message }, 404); } - const accommodation = this.accommodations.update(id, existing as never, { place_id, start_day_id, end_day_id, check_in, check_in_end, check_out, confirmation, notes } as never); + const { accommodation, mirror } = this.accommodations.update(id, existing as never, { place_id, start_day_id, end_day_id, check_in, check_in_end, check_out, confirmation, notes } as never); this.accommodations.broadcast(tripId, 'accommodation:updated', { accommodation }, socketId); - return { accommodation }; + this.accommodations.announceMirror(tripId, mirror, (event, payload) => this.accommodations.broadcast(tripId, event, payload, socketId), socketId); + return { accommodation, assignment: mirror.created, removedAssignments: mirror.removed }; } @RequirePermission('day_edit') @@ -111,7 +121,8 @@ export class AccommodationsController { if (!this.accommodations.get(id, tripId)) { throw new HttpException({ error: 'Accommodation not found' }, 404); } - const { linkedReservationIds, deletedBudgetItemIds } = this.accommodations.remove(id); + const { linkedReservationIds, deletedBudgetItemIds, mirror } = this.accommodations.remove(id); + this.accommodations.announceMirror(tripId, mirror, (event, payload) => this.accommodations.broadcast(tripId, event, payload, socketId), socketId); for (const reservationId of linkedReservationIds) { this.accommodations.broadcast(tripId, 'reservation:deleted', { reservationId }, socketId); } @@ -119,6 +130,6 @@ export class AccommodationsController { this.accommodations.broadcast(tripId, 'budget:deleted', { itemId }, socketId); } this.accommodations.broadcast(tripId, 'accommodation:deleted', { accommodationId: Number(id) }, socketId); - return { success: true }; + return { success: true, removedAssignments: mirror.removed }; } } diff --git a/server/src/nest/accommodations/accommodations.mcp.ts b/server/src/nest/accommodations/accommodations.mcp.ts index 3f1b6dbd9..06f0e54b2 100644 --- a/server/src/nest/accommodations/accommodations.mcp.ts +++ b/server/src/nest/accommodations/accommodations.mcp.ts @@ -9,7 +9,7 @@ import { AuthService } from '../auth/auth.service'; import { PlacesService } from '../places/places.service'; import { noAccess, permissionDenied } from '../../mcp/tools/_shared'; import { DatabaseService } from '../database/database.service'; -import { AccommodationsService } from './accommodations.service'; +import { AccommodationsService, type MirrorSender } from './accommodations.service'; function parseId(value: string | string[]): number | null { const n = Number(Array.isArray(value) ? value[0] : value); @@ -36,9 +36,15 @@ export class AccommodationsMcp { private readonly guards: McpToolGuardsService, ) {} + /** The tools' own broadcast, handed to announceMirror so the day stop a booking + * writes is announced exactly as the REST route announces it. */ + private mirrorSender(tripId: number): MirrorSender { + return (event, payload) => this.guards.safeBroadcast(tripId, event, payload as Record); + } + @Tool({ name: 'create_accommodation', - description: 'Add an accommodation (hotel, Airbnb, etc.) to a trip, linked to a place and a date range.', + description: 'Add an accommodation (hotel, Airbnb, etc.) to a trip, linked to a place and a date range. This also puts the place on its check-in day, so the stay shows up as a stop on the route.', inputSchema: { tripId: z.number().int().positive(), place_id: z.number().int().positive().describe('The place to use as the accommodation'), @@ -65,14 +71,15 @@ export class AccommodationsMcp { if (!this.guards.hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied(); const errors = this.accommodations.validateAccommodationRefs(tripId, place_id, start_day_id, end_day_id); if (errors.length > 0) return { content: [{ type: 'text' as const, text: errors.map(e => e.message).join(', ') }], isError: true }; - const accommodation = this.accommodations.createAccommodation(tripId, { place_id, start_day_id, end_day_id, check_in, check_in_end, check_out, confirmation, notes }); + const { accommodation, mirror } = this.accommodations.createAccommodation(tripId, { place_id, start_day_id, end_day_id, check_in, check_in_end, check_out, confirmation, notes }); this.guards.safeBroadcast(tripId, 'accommodation:created', { accommodation }); - return ok({ accommodation }); + this.accommodations.announceMirror(tripId, mirror, this.mirrorSender(tripId)); + return ok({ accommodation, assignment: mirror.created }); } @Tool({ name: 'create_place_accommodation', - description: 'Create a new place and immediately set it as an accommodation for a date range in one atomic operation. Use place details from search_place results. Only use when the place does not yet exist — if it already exists, use create_accommodation directly. Set price + currency to record the accommodation cost so it shows on the item.', + description: 'Create a new place, set it as an accommodation for a date range and put it on its check-in day, in one atomic operation. Use place details from search_place results. Only use when the place does not yet exist — if it already exists, use create_accommodation directly. Set price + currency to record the accommodation cost so it shows on the item.', inputSchema: { tripId: z.number().int().positive(), name: z.string().min(1).max(200), @@ -118,12 +125,16 @@ export class AccommodationsMcp { try { const result = this.db.transaction(() => { const place = this.places.create(String(tripId), { name, description, lat, lng, address, category_id, google_place_id, google_ftid, osm_id, notes: place_notes, website, phone, price, currency }); - const accommodation = this.accommodations.createAccommodation(tripId, { place_id: place.id, start_day_id, end_day_id, check_in, check_in_end, check_out, confirmation, notes: accommodation_notes }); - return { place, accommodation }; + const { accommodation, mirror } = this.accommodations.createAccommodation(tripId, { place_id: place.id, start_day_id, end_day_id, check_in, check_in_end, check_out, confirmation, notes: accommodation_notes }); + return { place, accommodation, mirror }; }); - this.guards.safeBroadcast(tripId, 'place:created', { place: result.place }); + // The stamped copy, not the one create() returned: booking the night types the + // place as lodging, and place:created is the only announcement it gets here. + const place = result.mirror.stamped ?? result.place; + this.guards.safeBroadcast(tripId, 'place:created', { place }); this.guards.safeBroadcast(tripId, 'accommodation:created', { accommodation: result.accommodation }); - return ok(result); + this.accommodations.announceMirror(tripId, { ...result.mirror, stamped: null }, this.mirrorSender(tripId)); + return ok({ place, accommodation: result.accommodation, assignment: result.mirror.created }); } catch { return { content: [{ type: 'text' as const, text: 'Failed to create place and accommodation.' }], isError: true }; } @@ -131,7 +142,7 @@ export class AccommodationsMcp { @Tool({ name: 'update_accommodation', - description: 'Update fields on an existing accommodation.', + description: 'Update fields on an existing accommodation. Moving it to another check-in day or place moves the stop it put on the route with it.', inputSchema: { tripId: z.number().int().positive(), accommodationId: z.number().int().positive(), @@ -159,14 +170,15 @@ export class AccommodationsMcp { if (!this.guards.hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied(); const existing = this.accommodations.getAccommodation(accommodationId, tripId); if (!existing) return { content: [{ type: 'text' as const, text: 'Accommodation not found.' }], isError: true }; - const accommodation = this.accommodations.updateAccommodation(accommodationId, existing, { place_id, start_day_id, end_day_id, check_in, check_in_end, check_out, confirmation, notes }); + const { accommodation, mirror } = this.accommodations.updateAccommodation(accommodationId, existing, { place_id, start_day_id, end_day_id, check_in, check_in_end, check_out, confirmation, notes }); this.guards.safeBroadcast(tripId, 'accommodation:updated', { accommodation }); - return ok({ accommodation }); + this.accommodations.announceMirror(tripId, mirror, this.mirrorSender(tripId)); + return ok({ accommodation, assignment: mirror.created }); } @Tool({ name: 'delete_accommodation', - description: 'Delete an accommodation from a trip.', + description: 'Delete an accommodation from a trip. The stop this booking put on its check-in day goes with it; a stop that was already there stays.', inputSchema: { tripId: z.number().int().positive(), accommodationId: z.number().int().positive(), @@ -181,9 +193,10 @@ export class AccommodationsMcp { if (!this.accommodations.getAccommodation(accommodationId, tripId)) return { content: [{ type: 'text' as const, text: 'Accommodation not found.' }], isError: true }; // linkedReservationId stays the first one so the tool's answer keeps its shape; // linkedReservationIds carries the rest for a block that had more than one booking. - const { linkedReservationId, linkedReservationIds } = this.accommodations.deleteAccommodation(accommodationId); + const { linkedReservationId, linkedReservationIds, mirror } = this.accommodations.deleteAccommodation(accommodationId); + this.accommodations.announceMirror(tripId, mirror, this.mirrorSender(tripId)); this.guards.safeBroadcast(tripId, 'accommodation:deleted', { id: accommodationId, linkedReservationId, linkedReservationIds }); - return ok({ success: true, linkedReservationId, linkedReservationIds }); + return ok({ success: true, linkedReservationId, linkedReservationIds, removedAssignments: mirror.removed }); } @ResourceTemplate({ diff --git a/server/src/nest/accommodations/accommodations.module.ts b/server/src/nest/accommodations/accommodations.module.ts index 96e41b831..4c66f125c 100644 --- a/server/src/nest/accommodations/accommodations.module.ts +++ b/server/src/nest/accommodations/accommodations.module.ts @@ -4,6 +4,7 @@ import { AccommodationsService } from './accommodations.service'; import { AccommodationsRpc } from './accommodations.rpc'; import { AccommodationsMcp } from './accommodations.mcp'; import { PlacesModule } from '../places/places.module'; +import { AssignmentsDomainModule } from '../assignments/assignments-domain.module'; import { AuthModule } from '../auth/auth.module'; import { DatabaseModule } from '../database/database.module'; import { PermissionsModule } from '../permissions/permissions.module'; @@ -20,9 +21,15 @@ import { McpSharedModule } from '../mcp-shared/mcp-shared.module'; * which is a foreign key, not a module edge. PlacesModule is here only because * create_place_accommodation writes a place and a stay in one transaction -- * the same reason DaysMcp could drop it. + * + * AssignmentsDomainModule is the service half only, the same leaf PlacesModule + * already imports, so the day stop a booking implies is written through + * AssignmentsService instead of a second copy of its SQL. No edge back: that + * module reaches permissions, query helpers, journey and realtime, none of + * which comes near accommodations. */ @Module({ - imports: [McpSharedModule, PermissionsModule, RealtimeModule, PluginGuardsModule, DatabaseModule, PlacesModule, AuthModule], + imports: [McpSharedModule, PermissionsModule, RealtimeModule, PluginGuardsModule, DatabaseModule, PlacesModule, AssignmentsDomainModule, AuthModule], controllers: [AccommodationsController], providers: [AccommodationsService, AccommodationsRpc, AccommodationsMcp], exports: [AccommodationsService], diff --git a/server/src/nest/accommodations/accommodations.rpc.ts b/server/src/nest/accommodations/accommodations.rpc.ts index b9da94d6c..e5375038e 100644 --- a/server/src/nest/accommodations/accommodations.rpc.ts +++ b/server/src/nest/accommodations/accommodations.rpc.ts @@ -30,7 +30,8 @@ type AccommodationInput = { * * Everything delegates to AccommodationsService, so the auto-created partner hotel reservation, * the metadata sync on update and the delete cascade behave exactly like the - * accommodations REST controller, cascade broadcasts included. + * accommodations REST controller, cascade broadcasts included -- the day stop a booking + * puts on the route among them. */ @PluginController() export class AccommodationsRpc { @@ -55,7 +56,7 @@ export class AccommodationsRpc { // Verifies the place and both days belong to this trip. const errors = this.days.validateAccommodationRefs(tripId, placeId, startDayId, endDayId); if (errors.length > 0) throw new ForbiddenResource(errors[0].message); - const accommodation = this.days.createAccommodation(tripId, { + const { accommodation, mirror } = this.days.createAccommodation(tripId, { place_id: placeId, start_day_id: startDayId, end_day_id: endDayId, @@ -68,6 +69,7 @@ export class AccommodationsRpc { this.realtime.broadcast(tripId, 'accommodation:created', { accommodation }); // The block creates a partner hotel reservation, so the bookings view refreshes too. this.realtime.broadcast(tripId, 'reservation:created', {}); + this.days.announceMirror(tripId, mirror, (event, payload) => this.realtime.broadcast(tripId, event, payload)); return accommodation; } @@ -84,8 +86,9 @@ export class AccommodationsRpc { const input = parsed.data as { place_id?: number; start_day_id?: number; end_day_id?: number; check_in?: string; check_in_end?: string; check_out?: string; confirmation?: string; notes?: string }; const errors = this.days.validateAccommodationRefs(tripId, input.place_id, input.start_day_id, input.end_day_id); if (errors.length > 0) throw new ForbiddenResource(errors[0].message); - const accommodation = this.days.updateAccommodation(accommodationId, existing, input); + const { accommodation, mirror } = this.days.updateAccommodation(accommodationId, existing, input); this.realtime.broadcast(tripId, 'accommodation:updated', { accommodation }); + this.days.announceMirror(tripId, mirror, (event, payload) => this.realtime.broadcast(tripId, event, payload)); return accommodation; } @@ -99,7 +102,8 @@ export class AccommodationsRpc { throw new ForbiddenResource(`no accommodation ${accommodationId} on trip ${tripId}`); } // Deleting a block can take its partner reservation and budget item with it. - const { linkedReservationIds, deletedBudgetItemIds } = this.days.deleteAccommodation(accommodationId); + const { linkedReservationIds, deletedBudgetItemIds, mirror } = this.days.deleteAccommodation(accommodationId); + this.days.announceMirror(tripId, mirror, (event, payload) => this.realtime.broadcast(tripId, event, payload)); for (const reservationId of linkedReservationIds) this.realtime.broadcast(tripId, 'reservation:deleted', { reservationId }); for (const itemId of deletedBudgetItemIds) this.realtime.broadcast(tripId, 'budget:deleted', { itemId }); this.realtime.broadcast(tripId, 'accommodation:deleted', { accommodationId }); diff --git a/server/src/nest/accommodations/accommodations.service.ts b/server/src/nest/accommodations/accommodations.service.ts index 4de369334..dd086bffc 100644 --- a/server/src/nest/accommodations/accommodations.service.ts +++ b/server/src/nest/accommodations/accommodations.service.ts @@ -1,12 +1,30 @@ import { Injectable } from '@nestjs/common'; import type { TrekWsPayload, TrekWsTripEventName } from '@trek/shared'; import { RealtimeService } from '../realtime/realtime.service'; -import { DatabaseService, type TripAccess } from '../database/database.service'; +import { DatabaseService, type PlaceWithTags, type TripAccess } from '../database/database.service'; import { PermissionsService } from '../permissions/permissions.service'; +import { AssignmentsService } from '../assignments/assignments.service'; import type { User } from '../../types'; type Trip = TripAccess; +type MirroredAssignment = ReturnType; + +/** How a surface sends the mirror's events; see announceMirror. */ +export type MirrorSender = (event: E, payload: TrekWsPayload) => void; + +/** What a stay write did to the day plan, on top of writing the stay itself. */ +export interface AccommodationMirror { + /** The day stop the booking added, or null when that day already held the place. */ + created: MirroredAssignment | null; + /** Day stops the booking took back, because it moved days or was deleted. */ + removed: { id: number; dayId: number }[]; + /** The place, when this write was the one that typed it as lodging. */ + stamped: PlaceWithTags | null; +} + +const noMirror = (): AccommodationMirror => ({ created: null, removed: [], stamped: null }); + export interface DayAccommodation { id: number; trip_id: number; @@ -43,6 +61,16 @@ export interface CreateAccommodationData { * not the thing doing the carrying. * * Still gated by 'day_edit', the same permission as days. + * + * It also owns the day stop a booking implies. A road-trip stop IS a day + * assignment: the rail, the server-side plan and the map all build their stops + * from day_assignments and only look the stay up afterwards, to hang check-in + * and check-out on one. A stay written without an assignment is therefore + * invisible to every routing surface, which is what made people enter their + * hotel a second time as an ordinary place. The road-trip side has always + * written both halves in one go (the place, its day, and the stay); this is the + * day planner catching up, in the domain that writes the stay, so the next + * surface to book a night does not have to remember it. */ @Injectable() export class AccommodationsService { @@ -50,6 +78,7 @@ export class AccommodationsService { private readonly dbs: DatabaseService, private readonly permissions: PermissionsService, private readonly realtime: RealtimeService, + private readonly assignments: AssignmentsService, ) {} private get db() { @@ -98,6 +127,21 @@ export class AccommodationsService { return this.deleteAccommodation(id); } + /** + * Send what a stay write did to the day plan, and let the journey skeletons + * catch up the way an assignment route does. + * + * Takes the sender rather than broadcasting itself: REST and the plugin RPC + * hand their socket id in, the MCP tools tag their events, and the fan-out is + * the one part that must not exist in three copies. + */ + announceMirror(tripId: string | number, mirror: AccommodationMirror, send: MirrorSender, socketId?: string): void { + for (const stop of mirror.removed) send('assignment:deleted', { assignmentId: stop.id, dayId: stop.dayId }); + if (mirror.created) send('assignment:created', { assignment: mirror.created }); + if (mirror.stamped) send('place:updated', { place: mirror.stamped }); + if (mirror.created || mirror.removed.length > 0) this.assignments.reconcile(tripId, socketId); + } + // ------------------------------------------------------------------------- // Accommodation CRUD // ------------------------------------------------------------------------- @@ -141,12 +185,88 @@ export class AccommodationsService { return errors; } + /** + * Put the booking's check-in day on the map. + * + * Only the check-in day gets a stop, even for a fortnight's stay: that is the + * day you drive there, and it is exactly what the road-trip side writes for a + * night it books itself. The later nights ride on the stay row, which is where + * the rail reads check-out from. + * + * Runs inside the caller's transaction. + */ + private mirrorStay(accommodationId: number, placeId: number | null, dayId: number): AccommodationMirror { + const mirror = noMirror(); + // A stay can outlive its place (place_id is ON DELETE SET NULL) and the booking + // form writes stays that never had one. Nothing to put on the map then. + if (!placeId) return mirror; + + // 'hotel' is a service stop: it takes no number, stays out of the day's stop + // count and falls under the existing "show service stops in Days" switch. Without + // the stamp a booked night made here would look nothing like one booked in the + // road trip. Only ever filled in when it is empty: a type the traveller picked + // (a campsite, say) is theirs. + const place = this.db.get<{ stop_type: string | null }>('SELECT stop_type FROM places WHERE id = ?', placeId); + if (place && !place.stop_type) { + this.db.run("UPDATE places SET stop_type = 'hotel' WHERE id = ?", placeId); + mirror.stamped = this.db.getPlaceWithTags(placeId); + } + + // The road-trip flow assigns the place to the day and only then books the night. + // Claiming that row would make cancelling the booking delete a stop the traveller + // placed, so the booking rides along with it and marks nothing as its own. Same + // answer for a place already planned for that day by hand. + if (this.db.get('SELECT id FROM day_assignments WHERE day_id = ? AND place_id = ?', dayId, placeId)) return mirror; + + // Through AssignmentsService, so the stop lands at the end of the day with the + // order_index every other new assignment gets. Evening is where you arrive at a + // hotel, and the day planner has no drive to position it against anyway. + const created = this.assignments.createAssignment(dayId, placeId); + if (created) { + this.db.run('UPDATE day_assignments SET accommodation_id = ? WHERE id = ?', accommodationId, created.id); + mirror.created = created; + } + return mirror; + } + + /** The day stops this booking, and only this booking, put on the plan. */ + private ownStops(accommodationId: number) { + return this.db.all<{ id: number; day_id: number; place_id: number }>( + 'SELECT id, day_id, place_id FROM day_assignments WHERE accommodation_id = ?', accommodationId + ); + } + + /** + * Carry the mirrored stop over to wherever the booking now is. + * + * A booking that owns no stop gets one, which is how a stay made before any of + * this existed picks one up: re-save it once and it appears. The cost is that a + * mirrored stop somebody deleted by hand comes back the next time they touch the + * booking, which is the lesser of the two surprises. + * + * Runs inside the caller's transaction. + */ + private remirrorStay(accommodationId: number, placeId: number | null, dayId: number): AccommodationMirror { + const own = this.ownStops(accommodationId); + if (own.length === 1 && own[0].day_id === dayId && own[0].place_id === placeId) return noMirror(); + + const mirror = noMirror(); + for (const stop of own) { + this.db.run('DELETE FROM day_assignments WHERE id = ?', stop.id); + mirror.removed.push({ id: stop.id, dayId: stop.day_id }); + } + const fresh = this.mirrorStay(accommodationId, placeId, dayId); + mirror.created = fresh.created; + mirror.stamped = fresh.stamped; + return mirror; + } + createAccommodation(tripId: string | number, data: CreateAccommodationData) { const { place_id, start_day_id, end_day_id, check_in, check_in_end, check_out, confirmation, notes } = data; - // The stay and its partner hotel reservation are one logical write — - // atomic, so a failed reservation insert can't leave an orphan stay. - const accommodationId = this.db.transaction(() => { + // The stay, its partner hotel reservation and the day stop it implies are one + // logical write, and an atomic one, so a failed insert halfway can't leave an orphan. + const written = this.db.transaction(() => { const result = this.db.run( 'INSERT INTO day_accommodations (trip_id, place_id, start_day_id, end_day_id, check_in, check_in_end, check_out, confirmation, notes) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)', tripId, place_id, start_day_id, end_day_id, check_in || null, check_in_end || null, check_out || null, confirmation || null, notes || null @@ -170,10 +290,10 @@ export class AccommodationsService { Object.keys(meta).length > 0 ? JSON.stringify(meta) : null ); - return newId; + return { accommodationId: newId, mirror: this.mirrorStay(Number(newId), place_id ?? null, start_day_id) }; }); - return this.getAccommodationWithPlace(accommodationId); + return { accommodation: this.getAccommodationWithPlace(written.accommodationId), mirror: written.mirror }; } getAccommodation(id: string | number, tripId: string | number) { @@ -193,10 +313,15 @@ export class AccommodationsService { const newConfirmation = fields.confirmation !== undefined ? fields.confirmation : existing.confirmation; const newNotes = fields.notes !== undefined ? fields.notes : existing.notes; - this.db.run( - 'UPDATE day_accommodations SET place_id = ?, start_day_id = ?, end_day_id = ?, check_in = ?, check_in_end = ?, check_out = ?, confirmation = ?, notes = ? WHERE id = ?', - newPlaceId, newStartDayId, newEndDayId, newCheckIn, newCheckInEnd, newCheckOut, newConfirmation, newNotes, id - ); + // The stay row and the day stop that mirrors it describe the same booking, so a + // move that wrote only one of the two must not survive. + const mirror = this.db.transaction(() => { + this.db.run( + 'UPDATE day_accommodations SET place_id = ?, start_day_id = ?, end_day_id = ?, check_in = ?, check_in_end = ?, check_out = ?, confirmation = ?, notes = ? WHERE id = ?', + newPlaceId, newStartDayId, newEndDayId, newCheckIn, newCheckInEnd, newCheckOut, newConfirmation, newNotes, id + ); + return this.remirrorStay(Number(id), newPlaceId, newStartDayId); + }); // Sync check-in/out/confirmation to every linked reservation. The booking form // lets more than one hotel booking point at the same block and there is no @@ -212,7 +337,7 @@ export class AccommodationsService { JSON.stringify(meta), newConfirmation || null, res.id); } - return this.getAccommodationWithPlace(Number(id)); + return { accommodation: this.getAccommodationWithPlace(Number(id)), mirror }; } /** @@ -230,6 +355,7 @@ export class AccommodationsService { deletedBudgetItemId: number | null; linkedReservationIds: number[]; deletedBudgetItemIds: number[]; + mirror: AccommodationMirror; } { return this.db.transaction(() => { const linkedRes = this.db.all<{ id: number }>('SELECT id FROM reservations WHERE accommodation_id = ?', Number(id)); @@ -243,6 +369,15 @@ export class AccommodationsService { this.db.run('DELETE FROM reservations WHERE id = ?', res.id); } + // Only the stops this booking put there itself. A stop the traveller placed + // and then booked a night at keeps standing, which is how cancelling a night + // in the road trip has always behaved. + const mirror = noMirror(); + for (const stop of this.ownStops(Number(id))) { + this.db.run('DELETE FROM day_assignments WHERE id = ?', stop.id); + mirror.removed.push({ id: stop.id, dayId: stop.day_id }); + } + this.db.run('DELETE FROM day_accommodations WHERE id = ?', id); const linkedReservationIds = linkedRes.map(r => r.id); return { @@ -250,6 +385,7 @@ export class AccommodationsService { deletedBudgetItemId: deletedBudgetItemIds[0] ?? null, linkedReservationIds, deletedBudgetItemIds, + mirror, }; }); } diff --git a/server/src/nest/days/days.service.ts b/server/src/nest/days/days.service.ts index 202394eff..625720bca 100644 --- a/server/src/nest/days/days.service.ts +++ b/server/src/nest/days/days.service.ts @@ -365,6 +365,16 @@ export class DaysService { ELSE :date || SUBSTR(reservation_time, 11) END WHERE accommodation_id = :accId AND type = 'hotel' `); + // The day stop a booking wrote moves with it, the way its linked booking does. + // Left behind it would sit on a day the traveller no longer sleeps there, with + // nothing on screen to say why. Re-indexed to the end of the target day, because + // its old position belonged to a day it is leaving. + const moveStayStop = this.db.prepare(` + UPDATE day_assignments + SET day_id = :dayId, + order_index = COALESCE((SELECT MAX(order_index) FROM day_assignments WHERE day_id = :dayId), -1) + 1 + WHERE accommodation_id = :accId + `); for (const stay of stays) { const oldStartDate = prevDateByDayId.get(stay.start_day_id); @@ -375,6 +385,7 @@ export class DaysService { if (newStart && newEnd && newStart.day_number <= newEnd.day_number && (newStart.id !== stay.start_day_id || newEnd.id !== stay.end_day_id)) { updateStay.run(newStart.id, newEnd.id, stay.id); + moveStayStop.run({ dayId: newStart.id, accId: stay.id }); stay.start_day_id = newStart.id; } } diff --git a/server/src/nest/reservations/reservations.service.ts b/server/src/nest/reservations/reservations.service.ts index 28dce5c35..787681fe9 100644 --- a/server/src/nest/reservations/reservations.service.ts +++ b/server/src/nest/reservations/reservations.service.ts @@ -562,7 +562,14 @@ export class ReservationsService { let accommodationCreated = false; - // Auto-create accommodation for hotel reservations + // Auto-create accommodation for hotel reservations. + // + // Deliberately NOT through AccommodationsService, which would also put the + // place on its check-in day: this surface is gated on 'reservation_edit' and + // that stop is 'day_edit' work, and the two are configured separately. A + // booking clerk must not rewrite the day plan as a side effect. A hotel + // booked here therefore still reaches the route once somebody opens the stay + // and saves it. 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; diff --git a/server/tests/e2e/reservations.e2e.test.ts b/server/tests/e2e/reservations.e2e.test.ts index 7907ae5ca..30e91e563 100644 --- a/server/tests/e2e/reservations.e2e.test.ts +++ b/server/tests/e2e/reservations.e2e.test.ts @@ -170,6 +170,30 @@ describe('Reservations + accommodations e2e (real auth guard + temp SQLite, real expect(badRefs.body).toEqual({ error: 'Place not found' }); }); + it('201 create also puts the place on its check-in day, and the delete takes that stop back', async () => { + // The road-trip view builds its stops from day_assignments and only looks the stay + // up afterwards, so a booking without one never reaches the route: the complaint + // was having to enter the same hotel a second time as an ordinary place. + const placeId = Number(db.prepare('INSERT INTO places (trip_id, name) VALUES (?, ?)').run(tripId, 'Hotel Adlon').lastInsertRowid); + const dayId = Number(db.prepare('INSERT INTO days (trip_id, day_number, date) VALUES (?, 4, ?)').run(tripId, '2026-03-04').lastInsertRowid); + + const create = await request(server) + .post(`/api/trips/${tripId}/accommodations`) + .set('Cookie', sessionCookie(1)) + .send({ place_id: placeId, start_day_id: dayId, end_day_id: dayId }); + expect(create.status).toBe(201); + // In the answer, not only on the socket: the broadcast skips the sender. + expect(create.body.assignment).toMatchObject({ day_id: dayId, place_id: placeId }); + expect(db.prepare('SELECT stop_type FROM places WHERE id = ?').get(placeId)).toMatchObject({ stop_type: 'hotel' }); + + const del = await request(server) + .delete(`/api/trips/${tripId}/accommodations/${create.body.accommodation.id}`) + .set('Cookie', sessionCookie(1)); + expect(del.status).toBe(200); + expect(del.body.removedAssignments).toEqual([{ id: create.body.assignment.id, dayId }]); + expect(db.prepare('SELECT id FROM day_assignments WHERE day_id = ?').all(dayId)).toEqual([]); + }); + it('404 when trip not accessible (accommodations)', async () => { canAccessTrip.mockReturnValue(undefined); const res = await request(server).get(`/api/trips/${tripId}/accommodations`).set('Cookie', sessionCookie(1)); diff --git a/server/tests/helpers/accommodations-service.ts b/server/tests/helpers/accommodations-service.ts new file mode 100644 index 000000000..9cbc8dea5 --- /dev/null +++ b/server/tests/helpers/accommodations-service.ts @@ -0,0 +1,29 @@ +import type { Database } from 'better-sqlite3'; +import { AccommodationsService } from '../../src/nest/accommodations/accommodations.service'; +import { AssignmentsService } from '../../src/nest/assignments/assignments.service'; +import { DatabaseService } from '../../src/nest/database/database.service'; +import { JourneyDomainService } from '../../src/nest/journey/journey-domain.service'; +import { PermissionsService } from '../../src/nest/permissions/permissions.service'; +import { QueryHelpersService } from '../../src/nest/query-helpers/query-helpers.service'; +import { RealtimeService } from '../../src/nest/realtime/realtime.service'; +import { TrekPhotosRepository } from '../../src/nest/photos/trek-photos.repository'; + +/** + * AccommodationsService over a test connection. + * + * Booking a night also writes the day stop that puts it on the route, through a + * real AssignmentsService rather than a stub: the stop is a row these cases read + * back, and it has to come out of the same connection. Five collaborators deep is + * why this is a helper and not five copies across the suites. + */ +export function makeAccommodationsService(conn: Database): AccommodationsService { + const dbs = new DatabaseService(conn); + const permissions = new PermissionsService(dbs); + const realtime = new RealtimeService(); + const assignments = new AssignmentsService( + dbs, permissions, realtime, + new QueryHelpersService(dbs), + new JourneyDomainService(dbs, realtime, new TrekPhotosRepository(dbs)), + ); + return new AccommodationsService(dbs, permissions, realtime, assignments); +} diff --git a/server/tests/helpers/mcp-test-controllers.ts b/server/tests/helpers/mcp-test-controllers.ts index 19cc94e4e..ecaa1388f 100644 --- a/server/tests/helpers/mcp-test-controllers.ts +++ b/server/tests/helpers/mcp-test-controllers.ts @@ -168,7 +168,10 @@ export function createMcpTestRegistry(): McpRegistry { generalStorage, ); const reservationsService = new ReservationsService(dbService, permissionsService, budgetService, realtimeService, notificationsStub(), new ReservationsReadRepository(dbService)); - const accommodationsService = new AccommodationsService(dbService, permissionsService, realtimeService); + // One instance, four consumers: AssignmentsMcp, ReservationsMcp, PlacesMcp and + // AccommodationsService, which writes the day stop a booked night implies. + const assignmentsService = new AssignmentsService(dbService, permissionsService, realtimeService, queryHelpersService, journeyDomain); + const accommodationsService = new AccommodationsService(dbService, permissionsService, realtimeService, assignmentsService); const membersService = new TripMembersService(dbService, budgetService, new UserCleanupService(dbService, budgetService), permissionsService, realtimeService, notificationsStub()); const tripsService = new TripsService( dbService, @@ -192,8 +195,6 @@ export function createMcpTestRegistry(): McpRegistry { // one — against the same test DB, which is what makes the `when:` gates // answer truthfully here instead of against the process-wide singleton. const addonsService = new AddonsService(dbService); - // One instance, three consumers: AssignmentsMcp, ReservationsMcp and PlacesMcp. - const assignmentsService = new AssignmentsService(dbService, permissionsService, realtimeService, queryHelpersService, journeyDomain); // The two photo providers, shared by MemoriesMcp (which browses them) and by // the capture backfill JourneyMcp schedules after a provider photo is attached // (which asks them when and where it was taken). Built for real rather than diff --git a/server/tests/helpers/plugin-host.ts b/server/tests/helpers/plugin-host.ts index a9341fd56..3f36397de 100644 --- a/server/tests/helpers/plugin-host.ts +++ b/server/tests/helpers/plugin-host.ts @@ -110,7 +110,7 @@ export function createPluginRpcHostFactory(dbs: DatabaseService): PluginRpcHostF const notifications = makeNotificationsService(dbs, realtime); const llmConfig = new LlmConfigResolver(new SettingsService(dbs), dbs, addons); const oauth = new PluginOAuthService(dbs); - const accommodations = new AccommodationsService(dbs, permissions, realtime); + const accommodations = new AccommodationsService(dbs, permissions, realtime, assignments); const trips = new TripsService(dbs, reservations, days, permissions, budget, vacay, realtime, unsplash, generalStorage); const members = new TripMembersService(dbs, budget, new UserCleanupService(dbs, budget), permissions, realtime, notificationsStub()); const guards = new PluginGuards(dbs, permissions, addons); diff --git a/server/tests/unit/nest/accommodations.controller.test.ts b/server/tests/unit/nest/accommodations.controller.test.ts index a8f48dffd..d275e9e52 100644 --- a/server/tests/unit/nest/accommodations.controller.test.ts +++ b/server/tests/unit/nest/accommodations.controller.test.ts @@ -7,12 +7,15 @@ import type { User } from '../../../src/types'; const user = { id: 1, role: 'user', email: 'u@example.test' } as User; const trip = { user_id: 1 }; const refs = { place_id: 2, start_day_id: 10, end_day_id: 11 }; +/** A write that left the day plan alone, which is what most of these cases are about. */ +const noMirror = { created: null, removed: [], stamped: null }; function makeService(overrides: Partial = {}): AccommodationsService { return { verifyTripAccess: vi.fn().mockReturnValue(trip), canEdit: vi.fn().mockReturnValue(true), broadcast: vi.fn(), + announceMirror: vi.fn(), validateRefs: vi.fn().mockReturnValue([]), ...overrides, } as unknown as AccommodationsService; @@ -48,13 +51,24 @@ describe('AccommodationsController (parity with the legacy accommodations sub-ro }); it('creates and emits accommodation:created + reservation:created', () => { - const create = vi.fn().mockReturnValue({ id: 9 }); + const create = vi.fn().mockReturnValue({ accommodation: { id: 9 }, mirror: noMirror }); const broadcast = vi.fn(); const svc = makeService({ create, broadcast } as Partial); - expect(new AccommodationsController(svc).create(user, '5', refs, 'sock')).toEqual({ accommodation: { id: 9 } }); + expect(new AccommodationsController(svc).create(user, '5', refs, 'sock')).toEqual({ accommodation: { id: 9 }, assignment: null }); expect(broadcast).toHaveBeenCalledWith('5', 'accommodation:created', { accommodation: { id: 9 } }, 'sock'); expect(broadcast).toHaveBeenCalledWith('5', 'reservation:created', {}, 'sock'); }); + + it('ACC-CTL-001 hands the day stop back in the answer and announces it', () => { + // The broadcast skips the socket that sent the request, so the answer is the + // only way the session that booked the night learns about its own stop. + const stop = { id: 77, day_id: 10 }; + const create = vi.fn().mockReturnValue({ accommodation: { id: 9 }, mirror: { created: stop, removed: [], stamped: null } }); + const announceMirror = vi.fn(); + const svc = makeService({ create, announceMirror } as Partial); + expect(new AccommodationsController(svc).create(user, '5', refs, 'sock')).toEqual({ accommodation: { id: 9 }, assignment: stop }); + expect(announceMirror).toHaveBeenCalledWith('5', { created: stop, removed: [], stamped: null }, expect.any(Function), 'sock'); + }); }); describe('PUT /:id', () => { @@ -65,12 +79,23 @@ describe('AccommodationsController (parity with the legacy accommodations sub-ro it('updates and broadcasts', () => { const get = vi.fn().mockReturnValue({ id: 9 }); - const update = vi.fn().mockReturnValue({ id: 9, notes: 'x' }); + const update = vi.fn().mockReturnValue({ accommodation: { id: 9, notes: 'x' }, mirror: noMirror }); const broadcast = vi.fn(); const svc = makeService({ get, update, broadcast } as Partial); - expect(new AccommodationsController(svc).update(user, '5', '9', refs, 'sock')).toEqual({ accommodation: { id: 9, notes: 'x' } }); + expect(new AccommodationsController(svc).update(user, '5', '9', refs, 'sock')).toEqual({ accommodation: { id: 9, notes: 'x' }, assignment: null, removedAssignments: [] }); expect(broadcast).toHaveBeenCalledWith('5', 'accommodation:updated', { accommodation: { id: 9, notes: 'x' } }, 'sock'); }); + + it('ACC-CTL-002 a booking moved to another day reports the stop it took with it', () => { + const get = vi.fn().mockReturnValue({ id: 9 }); + const mirror = { created: { id: 78, day_id: 11 }, removed: [{ id: 77, dayId: 10 }], stamped: null }; + const update = vi.fn().mockReturnValue({ accommodation: { id: 9 }, mirror }); + const announceMirror = vi.fn(); + const svc = makeService({ get, update, announceMirror } as Partial); + expect(new AccommodationsController(svc).update(user, '5', '9', refs, 'sock')) + .toEqual({ accommodation: { id: 9 }, assignment: mirror.created, removedAssignments: mirror.removed }); + expect(announceMirror).toHaveBeenCalledWith('5', mirror, expect.any(Function), 'sock'); + }); }); describe('DELETE /:id', () => { @@ -83,11 +108,11 @@ describe('AccommodationsController (parity with the legacy accommodations sub-ro const get = vi.fn().mockReturnValue({ id: 9 }); const remove = vi.fn().mockReturnValue({ linkedReservationId: 4, deletedBudgetItemId: 7, - linkedReservationIds: [4], deletedBudgetItemIds: [7], + linkedReservationIds: [4], deletedBudgetItemIds: [7], mirror: noMirror, }); const broadcast = vi.fn(); const svc = makeService({ get, remove, broadcast } as Partial); - expect(new AccommodationsController(svc).remove(user, '5', '9', 'sock')).toEqual({ success: true }); + expect(new AccommodationsController(svc).remove(user, '5', '9', 'sock')).toEqual({ success: true, removedAssignments: [] }); expect(broadcast).toHaveBeenCalledWith('5', 'reservation:deleted', { reservationId: 4 }, 'sock'); expect(broadcast).toHaveBeenCalledWith('5', 'budget:deleted', { itemId: 7 }, 'sock'); expect(broadcast).toHaveBeenCalledWith('5', 'accommodation:deleted', { accommodationId: 9 }, 'sock'); @@ -97,7 +122,7 @@ describe('AccommodationsController (parity with the legacy accommodations sub-ro const get = vi.fn().mockReturnValue({ id: 9 }); const remove = vi.fn().mockReturnValue({ linkedReservationId: 4, deletedBudgetItemId: null, - linkedReservationIds: [4, 5], deletedBudgetItemIds: [], + linkedReservationIds: [4, 5], deletedBudgetItemIds: [], mirror: noMirror, }); const broadcast = vi.fn(); const svc = makeService({ get, remove, broadcast } as Partial); diff --git a/server/tests/unit/nest/accommodations.service.test.ts b/server/tests/unit/nest/accommodations.service.test.ts index 5a4af99fa..7447e10dd 100644 --- a/server/tests/unit/nest/accommodations.service.test.ts +++ b/server/tests/unit/nest/accommodations.service.test.ts @@ -17,7 +17,9 @@ const { testDb, dbMock } = vi.hoisted(() => { db, closeDb: () => {}, reinitialize: () => {}, - getPlaceWithTags: () => null, + // Real enough for the stop-type stamp a stay write makes: it announces the + // place it typed, and a null here would hide that the write happened at all. + getPlaceWithTags: (id: number | string) => db.prepare('SELECT * FROM places WHERE id = ?').get(id) ?? null, canAccessTrip: (tripId: any, userId: number) => db.prepare(` SELECT t.id, t.user_id FROM trips t @@ -37,21 +39,18 @@ vi.mock('../../../src/websocket', () => ({ broadcast })); import { createTables } from '../../../src/db/schema'; import { runMigrations } from '../../../src/db/migrations'; import { resetTestDb } from '../../helpers/test-db'; -import { createUser, createTrip, createDay, createPlace, createDayAccommodation, addTripMember } from '../../helpers/factories'; +import { createUser, createTrip, createDay, createPlace, createDayAccommodation, createDayAssignment, addTripMember } from '../../helpers/factories'; import { DatabaseService } from '../../../src/nest/database/database.service'; import { PermissionsService } from '../../../src/nest/permissions/permissions.service'; import { RealtimeService } from '../../../src/nest/realtime/realtime.service'; import { AccommodationsService } from '../../../src/nest/accommodations/accommodations.service'; +import { makeAccommodationsService } from '../../helpers/accommodations-service'; import { AccommodationsModule } from '../../../src/nest/accommodations/accommodations.module'; import { AccommodationsController } from '../../../src/nest/accommodations/accommodations.controller'; import { expectRegisteredProvider, expectRegisteredController } from '../../helpers/module-providers'; // Named `svc` so the moved cases read exactly as they did on DaysService. -const svc = new AccommodationsService( - new DatabaseService(testDb), - new PermissionsService(new DatabaseService(testDb)), - new RealtimeService(), -); +const svc = makeAccommodationsService(testDb); beforeAll(() => { createTables(testDb); @@ -119,7 +118,7 @@ describe('createAccommodation', () => { const day = createDay(testDb, trip.id) as any; const place = createPlace(testDb, trip.id, { name: 'Grand Hotel' }) as any; - const accom = svc.createAccommodation(trip.id, { + const { accommodation: accom } = svc.createAccommodation(trip.id, { place_id: place.id, start_day_id: day.id, end_day_id: day.id, @@ -137,7 +136,7 @@ describe('createAccommodation', () => { const day = createDay(testDb, trip.id) as any; const place = createPlace(testDb, trip.id, { name: 'City Hotel' }) as any; - const accom = svc.createAccommodation(trip.id, { + const { accommodation: accom } = svc.createAccommodation(trip.id, { place_id: place.id, start_day_id: day.id, end_day_id: day.id, }) as any; @@ -155,7 +154,7 @@ describe('createAccommodation', () => { const trip = createTrip(testDb, user.id); const day = createDay(testDb, trip.id) as any; - const accom = svc.createAccommodation(trip.id, { + const { accommodation: accom } = svc.createAccommodation(trip.id, { place_id: null as unknown as number, start_day_id: day.id, end_day_id: day.id, }) as any; @@ -190,12 +189,12 @@ describe('updateAccommodation', () => { const trip = createTrip(testDb, user.id); const day = createDay(testDb, trip.id) as any; const place = createPlace(testDb, trip.id, { name: 'Hotel' }) as any; - const accom = svc.createAccommodation(trip.id, { + const { accommodation: accom } = svc.createAccommodation(trip.id, { place_id: place.id, start_day_id: day.id, end_day_id: day.id, }) as any; const existing = svc.getAccommodation(accom.id, trip.id)!; - const updated = svc.updateAccommodation(accom.id, existing as any, { check_in: '16:00', check_out: '12:00' }) as any; + const { accommodation: updated } = svc.updateAccommodation(accom.id, existing as any, { check_in: '16:00', check_out: '12:00' }) as any; expect(updated).toBeDefined(); // Verify linked reservation metadata was synced @@ -211,7 +210,7 @@ describe('updateAccommodation', () => { const trip = createTrip(testDb, user.id); const day = createDay(testDb, trip.id) as any; const place = createPlace(testDb, trip.id, { name: 'Hotel' }) as any; - const accom = svc.createAccommodation(trip.id, { + const { accommodation: accom } = svc.createAccommodation(trip.id, { place_id: place.id, start_day_id: day.id, end_day_id: day.id, confirmation: 'ABC123', }) as any; @@ -234,7 +233,7 @@ describe('updateAccommodation', () => { const accom = createDayAccommodation(testDb, trip.id, place.id, day.id, day.id) as any; const existing = svc.getAccommodation(accom.id, trip.id)!; - const updated = svc.updateAccommodation(accom.id, existing as any, { check_in: '15:00' }) as any; + const { accommodation: updated } = svc.updateAccommodation(accom.id, existing as any, { check_in: '15:00' }) as any; expect(updated.check_in).toBe('15:00'); expect(testDb.prepare('SELECT COUNT(*) as n FROM reservations WHERE accommodation_id = ?').get(accom.id)).toMatchObject({ n: 0 }); @@ -248,7 +247,7 @@ describe('updateAccommodation', () => { const trip = createTrip(testDb, user.id); const day = createDay(testDb, trip.id) as any; const place = createPlace(testDb, trip.id, { name: 'Hotel' }) as any; - const accom = svc.createAccommodation(trip.id, { + const { accommodation: accom } = svc.createAccommodation(trip.id, { place_id: place.id, start_day_id: day.id, end_day_id: day.id, check_in: '15:00', }) as any; @@ -268,7 +267,7 @@ describe('updateAccommodation', () => { const trip = createTrip(testDb, user.id); const day = createDay(testDb, trip.id) as any; const place = createPlace(testDb, trip.id, { name: 'Hotel' }) as any; - const accom = svc.createAccommodation(trip.id, { + const { accommodation: accom } = svc.createAccommodation(trip.id, { place_id: place.id, start_day_id: day.id, end_day_id: day.id, }) as any; testDb.prepare('UPDATE reservations SET confirmation_number = ? WHERE accommodation_id = ?').run('RES-9', accom.id); @@ -287,7 +286,7 @@ describe('deleteAccommodation', () => { const trip = createTrip(testDb, user.id); const day = createDay(testDb, trip.id) as any; const place = createPlace(testDb, trip.id, { name: 'Hotel' }) as any; - const accom = svc.createAccommodation(trip.id, { + const { accommodation: accom } = svc.createAccommodation(trip.id, { place_id: place.id, start_day_id: day.id, end_day_id: day.id, }) as any; @@ -326,7 +325,7 @@ describe('deleteAccommodation', () => { const trip = createTrip(testDb, user.id); const day = createDay(testDb, trip.id) as any; const place = createPlace(testDb, trip.id, { name: 'Hotel' }) as any; - const accom = svc.createAccommodation(trip.id, { + const { accommodation: accom } = svc.createAccommodation(trip.id, { place_id: place.id, start_day_id: day.id, end_day_id: day.id, }) as any; @@ -342,6 +341,7 @@ describe('deleteAccommodation', () => { deletedBudgetItemId: budgetItemId, linkedReservationIds: [reservation.id], deletedBudgetItemIds: [budgetItemId], + mirror: { created: null, removed: [{ id: expect.any(Number), dayId: day.id }], stamped: null }, }); expect(testDb.prepare('SELECT id FROM budget_items WHERE id = ?').get(budgetItemId)).toBeUndefined(); }); @@ -354,7 +354,7 @@ describe('deleteAccommodation', () => { const trip = createTrip(testDb, user.id); const day = createDay(testDb, trip.id) as any; const place = createPlace(testDb, trip.id, { name: 'Hotel' }) as any; - const accom = svc.createAccommodation(trip.id, { + const { accommodation: accom } = svc.createAccommodation(trip.id, { place_id: place.id, start_day_id: day.id, end_day_id: day.id, }) as any; @@ -375,7 +375,7 @@ describe('deleteAccommodation', () => { const trip = createTrip(testDb, user.id); const day = createDay(testDb, trip.id) as any; const place = createPlace(testDb, trip.id, { name: 'Hotel' }) as any; - const accom = svc.createAccommodation(trip.id, { + const { accommodation: accom } = svc.createAccommodation(trip.id, { place_id: place.id, start_day_id: day.id, end_day_id: day.id, }) as any; testDb.prepare( @@ -412,9 +412,9 @@ describe('quirk fixes', () => { const trip = createTrip(testDb, user.id); const day = createDay(testDb, trip.id); const place = createPlace(testDb, trip.id, { name: 'Hotel' }); - const accom = svc.createAccommodation(trip.id, { + const { accommodation: accom } = svc.createAccommodation(trip.id, { place_id: place.id, start_day_id: day.id, end_day_id: day.id, - }) as { id: number }; + }) as { accommodation: { id: number } }; testDb.exec("CREATE TRIGGER boom BEFORE DELETE ON day_accommodations BEGIN SELECT RAISE(ABORT, 'boom'); END"); try { expect(() => svc.deleteAccommodation(accom.id)).toThrow(); @@ -462,7 +462,7 @@ describe('route-facing delegators', () => { const place = createPlace(testDb, trip.id, { name: 'Hotel' }) as any; // Both ids arrive from the route as strings. - const created = svc.create(String(trip.id), { + const { accommodation: created } = svc.create(String(trip.id), { place_id: place.id, start_day_id: day.id, end_day_id: day.id, confirmation: 'XY-1', }) as any; @@ -478,10 +478,10 @@ describe('route-facing delegators', () => { const trip = createTrip(testDb, user.id); const day = createDay(testDb, trip.id) as any; const place = createPlace(testDb, trip.id, { name: 'Hotel' }) as any; - const accom = svc.create(trip.id, { place_id: place.id, start_day_id: day.id, end_day_id: day.id }) as any; + const { accommodation: accom } = svc.create(trip.id, { place_id: place.id, start_day_id: day.id, end_day_id: day.id }) as any; const existing = svc.get(accom.id, trip.id)!; - const updated = svc.update(String(accom.id), existing as any, { check_in: '16:00', notes: 'late arrival' }) as any; + const { accommodation: updated } = svc.update(String(accom.id), existing as any, { check_in: '16:00', notes: 'late arrival' }) as any; expect(updated).toMatchObject({ check_in: '16:00', notes: 'late arrival' }); const reservation = testDb.prepare('SELECT metadata FROM reservations WHERE accommodation_id = ?').get(accom.id) as any; @@ -493,7 +493,7 @@ describe('route-facing delegators', () => { const trip = createTrip(testDb, user.id); const day = createDay(testDb, trip.id) as any; const place = createPlace(testDb, trip.id, { name: 'Hotel' }) as any; - const accom = svc.create(trip.id, { place_id: place.id, start_day_id: day.id, end_day_id: day.id }) as any; + const { accommodation: accom } = svc.create(trip.id, { place_id: place.id, start_day_id: day.id, end_day_id: day.id }) as any; const reservation = testDb.prepare('SELECT id FROM reservations WHERE accommodation_id = ?').get(accom.id) as any; // The controller broadcasts reservation:deleted off this return value. @@ -502,6 +502,7 @@ describe('route-facing delegators', () => { deletedBudgetItemId: null, linkedReservationIds: [reservation.id], deletedBudgetItemIds: [], + mirror: { created: null, removed: [{ id: expect.any(Number), dayId: day.id }], stamped: null }, }); expect(svc.get(accom.id, trip.id)).toBeUndefined(); }); @@ -580,6 +581,214 @@ describe('trip access and edit permission', () => { }); }); +/** + * Booking a night also puts its place on the check-in day, because that stop is what + * the road trip routes and the map draws. Reported from Discord: a hotel entered in + * the day planner never reached the road-trip view, so the same place had to be + * entered a second time as an ordinary stop. + */ +describe('the day stop a booking implies', () => { + const stopsOn = (dayId: number) => + testDb.prepare('SELECT id, place_id, order_index, accommodation_id FROM day_assignments WHERE day_id = ? ORDER BY order_index').all(dayId) as + { id: number; place_id: number; order_index: number; accommodation_id: number | null }[]; + + const book = (tripId: number, placeId: number | null, startDayId: number, endDayId: number) => + svc.createAccommodation(tripId, { place_id: placeId as number, start_day_id: startDayId, end_day_id: endDayId }) as any; + + it('ACC-021 the booked night puts its place on the check-in day and marks the stop as its own', () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id); + const place = createPlace(testDb, trip.id, { name: 'Hotel Adlon' }); + + const { accommodation, mirror } = book(trip.id, place.id, day.id, day.id); + + const stops = stopsOn(day.id); + expect(stops).toHaveLength(1); + expect(stops[0]).toMatchObject({ place_id: place.id, accommodation_id: accommodation.id }); + expect(mirror.created).toMatchObject({ id: stops[0].id, day_id: day.id, place_id: place.id }); + }); + + it('ACC-022 the stop lands at the end of the day, where you arrive at a hotel', () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id); + const museum = createPlace(testDb, trip.id, { name: 'Pergamon' }); + const hotel = createPlace(testDb, trip.id, { name: 'Hotel Adlon' }); + createDayAssignment(testDb, day.id, museum.id); + + book(trip.id, hotel.id, day.id, day.id); + + expect(stopsOn(day.id).map(a => a.place_id)).toEqual([museum.id, hotel.id]); + }); + + it('ACC-023 the place is typed as lodging, so the rail draws it as a service stop', () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id); + const place = createPlace(testDb, trip.id, { name: 'Hotel Adlon' }); + + const { mirror } = book(trip.id, place.id, day.id, day.id); + + expect(testDb.prepare('SELECT stop_type FROM places WHERE id = ?').get(place.id)).toMatchObject({ stop_type: 'hotel' }); + // Announced, or the places list keeps the stale null until a reload. + expect(mirror.stamped).toMatchObject({ id: place.id, stop_type: 'hotel' }); + // And the stop carries it, so the rail does not number it. + expect(mirror.created.place.stop_type).toBe('hotel'); + }); + + it('ACC-024 a stop type the traveller picked is never overwritten', () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id); + const place = createPlace(testDb, trip.id, { name: 'Camping Isar' }); + testDb.prepare("UPDATE places SET stop_type = 'campsite' WHERE id = ?").run(place.id); + + const { mirror } = book(trip.id, place.id, day.id, day.id); + + expect(testDb.prepare('SELECT stop_type FROM places WHERE id = ?').get(place.id)).toMatchObject({ stop_type: 'campsite' }); + expect(mirror.stamped).toBeNull(); + }); + + it('ACC-025 a place already planned for that day gets no second stop, and the booking claims neither', () => { + // This is the road-trip flow: it assigns the place to the day and only then books + // the night. A second stop would draw the hotel twice, and claiming the existing + // one would let cancelling the booking delete a stop the traveller placed. + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id); + const place = createPlace(testDb, trip.id, { name: 'Hotel Adlon' }); + const own = createDayAssignment(testDb, day.id, place.id); + + const { mirror } = book(trip.id, place.id, day.id, day.id); + + expect(stopsOn(day.id)).toEqual([expect.objectContaining({ id: own.id, accommodation_id: null })]); + expect(mirror.created).toBeNull(); + }); + + it('ACC-026 only the check-in day, not every night of a long stay', () => { + // You drive there once. The later nights ride on the stay row, which is where the + // rail reads check-out from. + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const arrive = createDay(testDb, trip.id); + const middle = createDay(testDb, trip.id); + const leave = createDay(testDb, trip.id); + const place = createPlace(testDb, trip.id, { name: 'Hotel Adlon' }); + + book(trip.id, place.id, arrive.id, leave.id); + + expect(stopsOn(arrive.id)).toHaveLength(1); + expect(stopsOn(middle.id)).toHaveLength(0); + expect(stopsOn(leave.id)).toHaveLength(0); + }); + + it('ACC-027 a stay with no place writes no stop instead of throwing', () => { + // day_accommodations.place_id is nullable (ON DELETE SET NULL) and the booking form + // writes stays that never had one. + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id); + + const { mirror } = book(trip.id, null, day.id, day.id); + + expect(mirror).toEqual({ created: null, removed: [], stamped: null }); + expect(stopsOn(day.id)).toHaveLength(0); + }); + + it('ACC-028 moving the booking to another day moves its stop with it', () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day1 = createDay(testDb, trip.id); + const day2 = createDay(testDb, trip.id); + const place = createPlace(testDb, trip.id, { name: 'Hotel Adlon' }); + const { accommodation } = book(trip.id, place.id, day1.id, day1.id); + const before = stopsOn(day1.id)[0]; + + const existing = svc.getAccommodation(accommodation.id, trip.id)!; + const { mirror } = svc.updateAccommodation(accommodation.id, existing, { start_day_id: day2.id, end_day_id: day2.id }) as any; + + expect(stopsOn(day1.id)).toHaveLength(0); + expect(stopsOn(day2.id)).toEqual([expect.objectContaining({ place_id: place.id, accommodation_id: accommodation.id })]); + expect(mirror.removed).toEqual([{ id: before.id, dayId: day1.id }]); + expect(mirror.created).toMatchObject({ day_id: day2.id }); + }); + + it('ACC-029 editing an unrelated field leaves the stop exactly where it is', () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id); + const place = createPlace(testDb, trip.id, { name: 'Hotel Adlon' }); + const { accommodation } = book(trip.id, place.id, day.id, day.id); + const before = stopsOn(day.id); + + const existing = svc.getAccommodation(accommodation.id, trip.id)!; + const { mirror } = svc.updateAccommodation(accommodation.id, existing, { check_in: '16:00' }) as any; + + expect(stopsOn(day.id)).toEqual(before); + expect(mirror).toEqual({ created: null, removed: [], stamped: null }); + }); + + it('ACC-030 a stay booked before any of this existed picks up its stop on the next save', () => { + // The deliberate alternative to rewriting everybody finished trips in a migration: + // the booking reaches the route the moment somebody opens it and saves. + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id); + const place = createPlace(testDb, trip.id, { name: 'Hotel Adlon' }); + const old = createDayAccommodation(testDb, trip.id, place.id, day.id, day.id) as any; + + const existing = svc.getAccommodation(old.id, trip.id)!; + svc.updateAccommodation(old.id, existing, { check_in: '15:00' }); + + expect(stopsOn(day.id)).toEqual([expect.objectContaining({ place_id: place.id, accommodation_id: old.id })]); + }); + + it('ACC-031 cancelling a booking takes back its own stop and leaves the other one', () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id); + const hotel = createPlace(testDb, trip.id, { name: 'Hotel Adlon' }); + const museum = createPlace(testDb, trip.id, { name: 'Pergamon' }); + const own = createDayAssignment(testDb, day.id, museum.id); + const { accommodation } = book(trip.id, hotel.id, day.id, day.id); + const mirrored = stopsOn(day.id).find(a => a.place_id === hotel.id)!; + + const { mirror } = svc.deleteAccommodation(accommodation.id); + + expect(stopsOn(day.id)).toEqual([expect.objectContaining({ id: own.id })]); + expect(mirror.removed).toEqual([{ id: mirrored.id, dayId: day.id }]); + }); + + it('ACC-032 cancelling a night booked in the road trip keeps the stop standing', () => { + // The rail turns a night back into a pause by deleting the stay alone; the stop is + // the thing being edited there and has to survive. + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id); + const place = createPlace(testDb, trip.id, { name: 'Hotel Adlon' }); + const own = createDayAssignment(testDb, day.id, place.id); + const { accommodation } = book(trip.id, place.id, day.id, day.id); + + const { mirror } = svc.deleteAccommodation(accommodation.id); + + expect(stopsOn(day.id)).toEqual([expect.objectContaining({ id: own.id })]); + expect(mirror.removed).toEqual([]); + }); + + it('ACC-033 announceMirror sends the removal before the arrival', () => { + // Order matters on a move: the day plan would briefly hold the place twice if the + // arrival went first. + const sent: string[] = []; + svc.announceMirror(5, { + created: { id: 78, day_id: 11 } as never, + removed: [{ id: 77, dayId: 10 }], + stamped: { id: 3 } as never, + }, event => { sent.push(event); }); + expect(sent).toEqual(['assignment:deleted', 'assignment:created', 'place:updated']); + }); +}); + describe('AccommodationsService wiring', () => { it('ACC-001: the module carries the controller, the service and the RPC surface', () => { expectRegisteredController(AccommodationsModule, AccommodationsController); diff --git a/server/tests/unit/nest/days.service.test.ts b/server/tests/unit/nest/days.service.test.ts index 1a3f08fec..466f65e8f 100644 --- a/server/tests/unit/nest/days.service.test.ts +++ b/server/tests/unit/nest/days.service.test.ts @@ -62,11 +62,11 @@ const bridgeGetDay = (id: string | number, tripId: string | number) => svc.getDa const bridgeListDays = (tripId: string | number) => svc.list(tripId); import { RealtimeService } from '../../../src/nest/realtime/realtime.service'; import { QueryHelpersService } from '../../../src/nest/query-helpers/query-helpers.service'; -import { AccommodationsService } from '../../../src/nest/accommodations/accommodations.service'; +import { makeAccommodationsService } from '../../helpers/accommodations-service'; import type { Day } from '../../../src/types'; const svc = new DaysService(new DatabaseService(testDb), new PermissionsService(new DatabaseService(testDb)), new RealtimeService(), new QueryHelpersService(new DatabaseService(testDb))); -const accommodations = new AccommodationsService(new DatabaseService(testDb), new PermissionsService(new DatabaseService(testDb)), new RealtimeService()); +const accommodations = makeAccommodationsService(testDb); beforeAll(() => { createTables(testDb); diff --git a/server/tests/unit/nest/trip-read-model.service.test.ts b/server/tests/unit/nest/trip-read-model.service.test.ts index feb0d046a..0e9a81607 100644 --- a/server/tests/unit/nest/trip-read-model.service.test.ts +++ b/server/tests/unit/nest/trip-read-model.service.test.ts @@ -66,6 +66,7 @@ import { UserCleanupService } from '../../../src/nest/auth/user-cleanup.service' import { TripMembersService } from '../../../src/nest/trip-members/trip-members.service'; import { TripReadModelService } from '../../../src/nest/trip-read-model/trip-read-model.service'; import { AccommodationsService } from '../../../src/nest/accommodations/accommodations.service'; +import { makeAccommodationsService } from '../../helpers/accommodations-service'; import { MapsService } from '../../../src/nest/maps/maps.service'; import { QueryHelpersService } from '../../../src/nest/query-helpers/query-helpers.service'; import { notificationsStub } from '../../helpers/notifications'; @@ -93,7 +94,7 @@ const placesSvc = new PlacesService( new JourneyDomainService(dbs(), new RealtimeService(), new TrekPhotosRepository(dbs())), makeStorageFixture('').storage, ); -const accommodationsSvc = new AccommodationsService(dbs(), new PermissionsService(dbs()), new RealtimeService()); +const accommodationsSvc = makeAccommodationsService(testDb); const membersSvc = new TripMembersService(dbs(), budgetSvc, new UserCleanupService(dbs(), budgetSvc), new PermissionsService(dbs()), new RealtimeService(), notificationsStub()); const buildReadModel = (database: DatabaseService, roster: TripMembersService = membersSvc) => diff --git a/server/tests/unit/nest/trips.service.test.ts b/server/tests/unit/nest/trips.service.test.ts index 16b8ad8ff..4e8a448bf 100644 --- a/server/tests/unit/nest/trips.service.test.ts +++ b/server/tests/unit/nest/trips.service.test.ts @@ -74,6 +74,7 @@ import { UserCleanupService } from '../../../src/nest/auth/user-cleanup.service' import { TripMembersService } from '../../../src/nest/trip-members/trip-members.service'; import { TripReadModelService } from '../../../src/nest/trip-read-model/trip-read-model.service'; import { AccommodationsService } from '../../../src/nest/accommodations/accommodations.service'; +import { makeAccommodationsService } from '../../helpers/accommodations-service'; import { MapsService } from '../../../src/nest/maps/maps.service'; import { UnsplashService } from '../../../src/nest/unsplash/unsplash.service'; import { PlacePhotoCacheService } from '../../../src/nest/place-photos/place-photo-cache.service'; @@ -112,7 +113,7 @@ const placesSvc = new PlacesService( new JourneyDomainService(dbs(), new RealtimeService(), new TrekPhotosRepository(dbs())), makeStorageFixture('').storage, ); -const accommodationsSvc = new AccommodationsService(dbs(), new PermissionsService(dbs()), new RealtimeService()); +const accommodationsSvc = makeAccommodationsService(testDb); const createAccommodation = accommodationsSvc.createAccommodation.bind(accommodationsSvc); const svc = new TripsService( @@ -449,9 +450,9 @@ describe('resyncAccommodationDays (#1288)', () => { const insertAccommodation = (tripId: number, startDayId: number, endDayId: number) => { const place = createPlace(testDb, tripId, { name: 'Grand Hotel' }); - const acc = createAccommodation(tripId, { + const { accommodation: acc } = createAccommodation(tripId, { place_id: place.id, start_day_id: startDayId, end_day_id: endDayId, - }) as { id: number }; + }) as { accommodation: { id: number } }; const linkedRes = testDb.prepare( 'SELECT id FROM reservations WHERE accommodation_id = ?', ).get(acc.id) as { id: number }; @@ -479,6 +480,22 @@ describe('resyncAccommodationDays (#1288)', () => { expect(res.reservation_time?.slice(0, 10)).toBe('2025-06-11'); }); + it('TRIP-SVC-059: the day stop a booking wrote follows it when the trip is re-dated', () => { + // Booking a night also puts its place on the check-in day. Re-dating the trip moves + // the stay to whichever day row now carries its date, and the stop has to go with + // it, or the route runs through a day the traveller is no longer staying on. + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id, { start_date: '2025-06-10', end_date: '2025-06-14' }); + const { accId } = insertAccommodation(trip.id, dayFor(trip.id, '2025-06-11'), dayFor(trip.id, '2025-06-13')); + const stopOf = () => testDb.prepare('SELECT day_id FROM day_assignments WHERE accommodation_id = ?').get(accId) as { day_id: number }; + expect(stopOf().day_id).toBe(dayFor(trip.id, '2025-06-11')); + + svc.updateTrip(trip.id, user.id, { start_date: '2025-06-09', end_date: '2025-06-14' }, 'user'); + + expect(stopOf().day_id).toBe(getAcc(accId).start_day_id); + expect(stopOf().day_id).toBe(dayFor(trip.id, '2025-06-11')); + }); + it('TRIP-SVC-036: moving the whole trip out of the old range keeps the accommodation glued to its days', () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id, { start_date: '2025-06-01', end_date: '2025-06-05' }); diff --git a/server/tests/unit/reservations/bookings.rpc.test.ts b/server/tests/unit/reservations/bookings.rpc.test.ts index 77226ae21..7f8741d5f 100644 --- a/server/tests/unit/reservations/bookings.rpc.test.ts +++ b/server/tests/unit/reservations/bookings.rpc.test.ts @@ -45,19 +45,25 @@ function build(opts: { canEdit?: boolean; cascade?: boolean; seenActions?: strin syncBudgetOnUpdate: vi.fn(), notifyBookingChange: vi.fn(), } as unknown as ReservationsService & Record>; + /** A stay write that left the day plan alone. */ + const noMirror = () => ({ created: null, removed: [], stamped: null }); // The lodging blocks live in AccommodationsService; AccommodationsRpc still calls its // injected copy `days`, which is why the fixture keeps that name. const days = { validateAccommodationRefs: vi.fn((_t: number, placeId?: number) => (placeId === 404 ? [{ message: 'place 404 is not on this trip' }] : [])), - createAccommodation: vi.fn(() => ({ id: 60 })), + createAccommodation: vi.fn(() => ({ accommodation: { id: 60 }, mirror: noMirror() })), getAccommodation: vi.fn((id: number) => (id === 11 ? { id: 11 } : undefined)), - updateAccommodation: vi.fn(() => ({ id: 11 })), + updateAccommodation: vi.fn(() => ({ accommodation: { id: 11 }, mirror: noMirror() })), deleteAccommodation: vi.fn(() => ({ linkedReservationId: opts.cascade ? 40 : null, deletedBudgetItemId: opts.cascade ? 7 : null, linkedReservationIds: opts.cascade ? [40] : [], deletedBudgetItemIds: opts.cascade ? [7] : [], + mirror: noMirror(), })), + // The real one broadcasts the day stop a booking writes; these cases are about + // the booking events, so the fixture keeps it inert and BOOK-RPC-016 covers it. + announceMirror: vi.fn(), } as unknown as AccommodationsService & Record>; const guards = new PluginGuards( { @@ -243,6 +249,14 @@ describe('AccommodationsRpc', () => { expect(f.days.updateAccommodation).not.toHaveBeenCalled(); }); + it('BOOK-RPC-017 the day stop a booking writes is announced through the plugin surface too', async () => { + const f = build(); + await f.host().dispatch(req('accommodations.create', { tripId: 1, input: { place_id: 7, start_day_id: 3, end_day_id: 4 } }), 42); + // Same call the REST route makes, so a plugin booking a night cannot leave the + // other sessions without the stop that puts it on the route. + expect(f.days.announceMirror).toHaveBeenCalledWith(1, { created: null, removed: [], stamped: null }, expect.any(Function)); + }); + it('BOOK-RPC-015d deleting a block without a partner cascades only its own event', async () => { const f = build(); await f.host().dispatch(req('accommodations.delete', { tripId: 1, accommodationId: 11 }), 42); diff --git a/wiki/Accommodations.md b/wiki/Accommodations.md index f617e333e..0b940d86c 100644 --- a/wiki/Accommodations.md +++ b/wiki/Accommodations.md @@ -33,6 +33,31 @@ For each day between the **From** day and **To** day (inclusive), the accommodat ![Day planner side bar with accomodation](assets/Hotel-ReservationDaySidebar.png) +## On the route + +Booking a night from the Day Detail panel also puts its place on the check-in day, as a +stop of its own. That stop is what the map draws a line to and what the Road Trip view +builds its route from, so the hotel now shows up there without having to be entered a +second time as an ordinary place. A night booked in the Road Trip view has always worked +this way; this makes the two directions match. + +A few details worth knowing: + +- Only the check-in day gets a stop, however many nights the stay runs. That is the day + you travel there; the later nights keep showing as badges in the day header. +- The place is marked as a **hotel** stop, which is why it carries no number in the Road + Trip rail and does not count towards the day's stop total. If you had already given the + place a stop type of your own, that one is kept. +- If the place was already planned for that day, nothing is added. You keep the stop you + placed, and the booking simply rides along with it. +- Moving the booking to a different check-in day moves its stop with it. Deleting the + booking removes the stop it created, and leaves a stop you placed yourself standing. +- Service stops, hotels among them, can be hidden from the day lists with **Show in Days + too** under **Service stops** in the Road Trip settings. + +Stays booked before this existed keep their day plans untouched. Open such a booking and +save it once and it picks up its stop. + ## In the day plan sidebar Accommodations appear as small colour-coded badges in the day header row of the day plan sidebar: