From c351a0471c893410588c378cdd2cae8c326c2021 Mon Sep 17 00:00:00 2001 From: Maurice Date: Sun, 13 Sep 2026 16:06:11 +0200 Subject: [PATCH] fix(accommodations): a check-in decides where in the day the night sits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The stop went to the end of the day, always, on the reasoning that a night ends the day. It usually does — but a check-in at eleven says the traveller is at the desk before whatever they pinned to the afternoon, and putting the stop behind it built a drive that reaches the hotel at seventeen past five after a sixteen hundred it was booked around. The order looked like the traveller's and was not; it was this code's default. A booking with a check-in now takes the place its hour asks for: ahead of the first stop pinned later, and still last when nothing is. Editing the check-in re-seats it, in both directions, read off the chain rather than recomputed — the index a fresh insert would get is not the index a row already holds. Stops without an hour of their own are passed over in either direction. Their place in the chain came from the traveller, not from a clock, and a night has no business pushing past one. --- .../accommodations/accommodations.service.ts | 80 ++++++++++++++++--- .../nest/assignments/assignments.service.ts | 11 ++- .../nest/reservations/reservations.service.ts | 6 +- .../unit/nest/accommodations.service.test.ts | 64 +++++++++++++++ 4 files changed, 143 insertions(+), 18 deletions(-) diff --git a/server/src/nest/accommodations/accommodations.service.ts b/server/src/nest/accommodations/accommodations.service.ts index 4e4f5d37e..4f7d7d090 100644 --- a/server/src/nest/accommodations/accommodations.service.ts +++ b/server/src/nest/accommodations/accommodations.service.ts @@ -147,13 +147,13 @@ export class AccommodationsService { // ------------------------------------------------------------------------- /** Put a freshly written stay on the map. */ - attachStayStop(accommodationId: number, placeId: number | null, dayId: number): AccommodationMirror { - return this.mirrorStay(accommodationId, placeId, dayId); + attachStayStop(accommodationId: number, placeId: number | null, dayId: number, checkIn?: string | null): AccommodationMirror { + return this.mirrorStay(accommodationId, placeId, dayId, checkIn); } /** Carry a stay's own stop over to where the stay now is. */ - moveStayStop(accommodationId: number, placeId: number | null, dayId: number): AccommodationMirror { - return this.remirrorStay(accommodationId, placeId, dayId); + moveStayStop(accommodationId: number, placeId: number | null, dayId: number, checkIn?: string | null): AccommodationMirror { + return this.remirrorStay(accommodationId, placeId, dayId, checkIn); } /** Take back the stops of a stay that is being deleted elsewhere. */ @@ -229,7 +229,53 @@ export class AccommodationsService { * * Runs inside the caller's transaction. */ - private mirrorStay(accommodationId: number, placeId: number | null, dayId: number): AccommodationMirror { + /** + * Where in the day the booked night belongs. + * + * Last, unless the booking names an hour. A night usually ends the day, which is why + * that is the default — but a check-in at eleven says the traveller is at the desk + * before whatever they pinned to the afternoon, and dropping the stop behind it built + * a drive that visits the hotel after the sixteen-hundred it was booked around. + * + * Only stops that carry a clock of their own are compared. One without a time takes + * its position from the chain, not from the hour, so pushing past it would move a + * stop the traveller placed deliberately. + */ + private positionForCheckIn(dayId: number, checkIn: string | null | undefined): number | undefined { + if (!checkIn) return undefined; + const rows = this.db.all<{ order_index: number; at: string | null }>(` + SELECT da.order_index, COALESCE(da.assignment_time, p.place_time) AS at + FROM day_assignments da JOIN places p ON p.id = da.place_id + WHERE da.day_id = ? ORDER BY da.order_index + `, dayId); + const later = rows.find(row => row.at !== null && row.at > checkIn); + return later?.order_index; + } + + /** + * Whether the night already sits where its check-in says it should. + * + * Read off the chain rather than recomputed, because the index a fresh insert would + * get is not the index this row already occupies. Two ways to be wrong: something + * with a later hour ahead of it, or something with an earlier one behind it. Stops + * without an hour are passed over in both directions — their place in the chain is + * the traveller's doing, not a clock's. + */ + private seatedByCheckIn(dayId: number, ownId: number, checkIn: string | null | undefined): boolean { + if (!checkIn) return true; + const rows = this.db.all<{ id: number; at: string | null }>(` + SELECT da.id, COALESCE(da.assignment_time, p.place_time) AS at + FROM day_assignments da JOIN places p ON p.id = da.place_id + WHERE da.day_id = ? ORDER BY da.order_index + `, dayId); + const own = rows.findIndex(row => row.id === ownId); + if (own < 0) return true; + const laterAhead = rows.slice(0, own).some(row => row.at !== null && row.at > checkIn); + const earlierBehind = rows.slice(own + 1).some(row => row.at !== null && row.at <= checkIn); + return !laterAhead && !earlierBehind; + } + + private mirrorStay(accommodationId: number, placeId: number | null, dayId: number, checkIn?: string | null): 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. @@ -261,14 +307,17 @@ export class AccommodationsService { // would leave that copy without it. The day list has nothing else to tell the // stop from a place the traveller added, so it would show the hotel a second // time until the next reload. - mirror.created = this.assignments.createAssignment(dayId, placeId, null, { accommodationId }); + mirror.created = this.assignments.createAssignment(dayId, placeId, null, { + accommodationId, + orderIndex: this.positionForCheckIn(dayId, checkIn), + }); 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 + return this.db.all<{ id: number; day_id: number; place_id: number; order_index: number }>( + 'SELECT id, day_id, place_id, order_index FROM day_assignments WHERE accommodation_id = ?', accommodationId ); } @@ -311,17 +360,22 @@ export class AccommodationsService { * * Runs inside the caller's transaction. */ - private remirrorStay(accommodationId: number, placeId: number | null, dayId: number): AccommodationMirror { + private remirrorStay(accommodationId: number, placeId: number | null, dayId: number, checkIn?: string | null): AccommodationMirror { const own = this.ownStops(accommodationId); if (own.length === 0) return noMirror(); - if (own.length === 1 && own[0].day_id === dayId && own[0].place_id === placeId) return noMirror(); + if (own.length === 1 && own[0].day_id === dayId && own[0].place_id === placeId) { + // Same place, same day — but a check-in moved to a different hour can still put + // the night somewhere else in the chain, and a stop sitting after the afternoon + // it was booked around is the plan reading back wrong. + if (this.seatedByCheckIn(dayId, own[0].id, checkIn)) 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); + const fresh = this.mirrorStay(accommodationId, placeId, dayId, checkIn); mirror.created = fresh.created; mirror.stamped = fresh.stamped; return mirror; @@ -356,7 +410,7 @@ export class AccommodationsService { Object.keys(meta).length > 0 ? JSON.stringify(meta) : null ); - return { accommodationId: newId, mirror: this.mirrorStay(Number(newId), place_id ?? null, start_day_id) }; + return { accommodationId: newId, mirror: this.mirrorStay(Number(newId), place_id ?? null, start_day_id, check_in) }; }); return { accommodation: this.getAccommodationWithPlace(written.accommodationId), mirror: written.mirror }; @@ -386,7 +440,7 @@ export class AccommodationsService { '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); + return this.remirrorStay(Number(id), newPlaceId, newStartDayId, newCheckIn); }); // Sync check-in/out/confirmation to every linked reservation. The booking form diff --git a/server/src/nest/assignments/assignments.service.ts b/server/src/nest/assignments/assignments.service.ts index 8170ff50a..2f4e97cbc 100644 --- a/server/src/nest/assignments/assignments.service.ts +++ b/server/src/nest/assignments/assignments.service.ts @@ -131,10 +131,17 @@ export class AssignmentsService { * stop that reaches it without its booking id is one the day list cannot tell * from a place the traveller added, so it draws the hotel a second time. */ - createAssignment(dayId: string | number, placeId: unknown, notes?: string | null, opts: { accommodationId?: number } = {}) { + createAssignment(dayId: string | number, placeId: unknown, notes?: string | null, opts: { accommodationId?: number; orderIndex?: number } = {}) { const result = this.dbs.transaction(() => { const maxOrder = this.dbs.get<{ max: number | null }>('SELECT MAX(order_index) as max FROM day_assignments WHERE day_id = ?', dayId)!; - const orderIndex = (maxOrder.max !== null ? maxOrder.max : -1) + 1; + const end = (maxOrder.max !== null ? maxOrder.max : -1) + 1; + // Somewhere in the middle when the caller says so, which means everything from + // there on moves down. The end is still the default and still what every caller + // but one asks for. + const orderIndex = opts.orderIndex !== undefined ? Math.max(0, Math.min(opts.orderIndex, end)) : end; + if (orderIndex < end) { + this.dbs.run('UPDATE day_assignments SET order_index = order_index + 1 WHERE day_id = ? AND order_index >= ?', dayId, orderIndex); + } return this.dbs.run( 'INSERT INTO day_assignments (day_id, place_id, order_index, notes, accommodation_id) VALUES (?, ?, ?, ?, ?)', diff --git a/server/src/nest/reservations/reservations.service.ts b/server/src/nest/reservations/reservations.service.ts index 0c371e685..a2f43e86b 100644 --- a/server/src/nest/reservations/reservations.service.ts +++ b/server/src/nest/reservations/reservations.service.ts @@ -604,7 +604,7 @@ export class ReservationsService { // night entered under Days. Without it the hotel booked on this form is the // one place the drive does not know about, which is the duplicate entry this // whole change exists to remove. - stayMirror = this.accommodations.attachStayStop(resolvedAccommodationId, accPlaceId || null, start_day_id); + stayMirror = this.accommodations.attachStayStop(resolvedAccommodationId, accPlaceId || null, start_day_id, check_in); } } @@ -748,14 +748,14 @@ export class ReservationsService { // The stay just moved. Its stop moves with it, or it is left sitting on a // day nobody sleeps there any more, hidden from the day list because it // still carries this booking's id and stranded in the middle of the drive. - stayMirror = this.accommodations.moveStayStop(resolvedAccId, accPlaceId || null, start_day_id); + stayMirror = this.accommodations.moveStayStop(resolvedAccId, accPlaceId || null, start_day_id, check_in); } 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); - stayMirror = this.accommodations.attachStayStop(resolvedAccId, accPlaceId, start_day_id); + stayMirror = this.accommodations.attachStayStop(resolvedAccId, accPlaceId, start_day_id, check_in); } accommodationChanged = true; } diff --git a/server/tests/unit/nest/accommodations.service.test.ts b/server/tests/unit/nest/accommodations.service.test.ts index 1e7f24c74..243a6ece7 100644 --- a/server/tests/unit/nest/accommodations.service.test.ts +++ b/server/tests/unit/nest/accommodations.service.test.ts @@ -628,6 +628,70 @@ describe('the day stop a booking implies', () => { expect(stopsOn(day.id).map(a => a.place_id)).toEqual([museum.id, hotel.id]); }); + it('ACC-022b a check-in puts the night before whatever is pinned to a later hour', () => { + // The default is last, because a night usually ends the day. A check-in at eleven + // says otherwise, and dropping the stop behind a stop pinned to the afternoon built + // a drive that reaches the hotel after the hour it was booked around. + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id); + const afternoon = createPlace(testDb, trip.id, { name: 'Mercure' }); + const hotel = createPlace(testDb, trip.id, { name: 'Billstedt' }); + const pinned = createDayAssignment(testDb, day.id, afternoon.id); + testDb.prepare("UPDATE day_assignments SET assignment_time = '16:00' WHERE id = ?").run(pinned.id); + + svc.createAccommodation(trip.id, { place_id: hotel.id, start_day_id: day.id, end_day_id: day.id, check_in: '11:00' }); + + expect(stopsOn(day.id).map(a => a.place_id)).toEqual([hotel.id, afternoon.id]); + }); + + it('ACC-022c a check-in after everything pinned stays last, like a night without one', () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id); + const morning = createPlace(testDb, trip.id, { name: 'Museum' }); + const hotel = createPlace(testDb, trip.id, { name: 'Billstedt' }); + const pinned = createDayAssignment(testDb, day.id, morning.id); + testDb.prepare("UPDATE day_assignments SET assignment_time = '09:00' WHERE id = ?").run(pinned.id); + + svc.createAccommodation(trip.id, { place_id: hotel.id, start_day_id: day.id, end_day_id: day.id, check_in: '18:00' }); + + expect(stopsOn(day.id).map(a => a.place_id)).toEqual([morning.id, hotel.id]); + }); + + it('ACC-022d a stop without an hour of its own is never pushed past', () => { + // Its position came from the chain, not from a clock, so the traveller put it there. + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id); + const untimed = createPlace(testDb, trip.id, { name: 'Hafen' }); + const hotel = createPlace(testDb, trip.id, { name: 'Billstedt' }); + createDayAssignment(testDb, day.id, untimed.id); + + svc.createAccommodation(trip.id, { place_id: hotel.id, start_day_id: day.id, end_day_id: day.id, check_in: '11:00' }); + + expect(stopsOn(day.id).map(a => a.place_id)).toEqual([untimed.id, hotel.id]); + }); + + it('ACC-022e moving the check-in later re-seats the night', () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id); + const afternoon = createPlace(testDb, trip.id, { name: 'Mercure' }); + const hotel = createPlace(testDb, trip.id, { name: 'Billstedt' }); + const pinned = createDayAssignment(testDb, day.id, afternoon.id); + testDb.prepare("UPDATE day_assignments SET assignment_time = '16:00' WHERE id = ?").run(pinned.id); + const { accommodation } = svc.createAccommodation(trip.id, { + place_id: hotel.id, start_day_id: day.id, end_day_id: day.id, check_in: '11:00', + }) as any; + expect(stopsOn(day.id).map(a => a.place_id)).toEqual([hotel.id, afternoon.id]); + + const existing = svc.getAccommodation(accommodation.id, trip.id)!; + svc.updateAccommodation(accommodation.id, existing, { check_in: '20:00' }); + + expect(stopsOn(day.id).map(a => a.place_id)).toEqual([afternoon.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);