diff --git a/client/src/components/Roadtrip/RoadtripSidebar.tsx b/client/src/components/Roadtrip/RoadtripSidebar.tsx index 222f2e7bc..f80292918 100644 --- a/client/src/components/Roadtrip/RoadtripSidebar.tsx +++ b/client/src/components/Roadtrip/RoadtripSidebar.tsx @@ -1553,12 +1553,16 @@ function DaySection({ day, selectedAssignmentId, onSelectStop, onReorderStop, on {formatDate(day.date, language)} ) : null} - - {t('roadtrip.leg.driveText', { - distance: formatDistance(day.distance / 1000, distanceUnit), - time: formatDurationShort(day.duration), - })} - + {/* A day that is only the night booked for it has no drive to sum up and no + stop to count; the hotel row below says everything it has to say. */} + {day.legs.length > 0 ? ( + + {t('roadtrip.leg.driveText', { + distance: formatDistance(day.distance / 1000, distanceUnit), + time: formatDurationShort(day.duration), + })} + + ) : null} {day.dayWarning ? ( @@ -1569,9 +1573,11 @@ function DaySection({ day, selectedAssignmentId, onSelectStop, onReorderStop, on ) : null} - - {t('roadtrip.day.stopCount', { count: day.stops.filter(s => !s.automaticNight && !isServiceStopType(s.stopType)).length })} - + {day.legs.length > 0 ? ( + + {t('roadtrip.day.stopCount', { count: day.stops.filter(s => !s.automaticNight && !isServiceStopType(s.stopType)).length })} + + ) : null} {/* The other half of "where possible". The setting is a weighting, so a day with no untolled crossing comes back on the toll road — and the only thing worse than not avoiding it is not avoiding it silently, which reads as the diff --git a/client/src/components/Roadtrip/useRoadtripRoutes.ts b/client/src/components/Roadtrip/useRoadtripRoutes.ts index f185063c0..4ca36efb4 100644 --- a/client/src/components/Roadtrip/useRoadtripRoutes.ts +++ b/client/src/components/Roadtrip/useRoadtripRoutes.ts @@ -1,5 +1,5 @@ import { useRoadtripSettings } from '../../hooks/useRoadtripSettings' -import { assembleRoadtrip, foldRouteRun, type RoadtripStop, type RoadtripRoutes, type PlanDay, type QuietDay, type RoutedLeg } from '@trek/shared/roadtrip' +import { assembleRoadtrip, foldRouteRun, standsAsDay, type RoadtripStop, type RoadtripRoutes, type PlanDay, type QuietDay, type RoutedLeg } from '@trek/shared/roadtrip' import { useEffect, useMemo, useRef, useState } from 'react' import { calculateRouteWithLegs, RoutingRefusedError } from '../Map/RouteCalculator' import { resolveLegMode } from '../Planner/legMode' @@ -91,6 +91,7 @@ const asStop = (a: Assignment, ownerDayId: number, ownerIndex: number, accommoda // traveller's own statement about this stop, and the stay then runs until it. leaveAt: a.assignment_end_time ?? p.end_time ?? null, checkInTime: stay?.check_in ?? null, + night: stay !== undefined, dwellMinutes: typeof p.duration_minutes === 'number' ? p.duration_minutes : null, endDay: a.end_day === true, legMode: a.leg_transport_mode ?? null, @@ -222,7 +223,7 @@ export function useRoadtripRoutes( stops, } }) - .filter(d => d.stops.length > 1) + .filter(d => standsAsDay(d.stops)) }, [days, assignments, accommodations]) const quietDays = useMemo(() => { @@ -240,7 +241,7 @@ export function useRoadtripRoutes( .filter((s): s is RoadtripStop => s !== null) .map((s, i) => ({ ...s, ownerIndex: i })), })) - .filter(d => d.stops.length < 2) + .filter(d => !standsAsDay(d.stops)) }, [days, assignments, accommodations]) // Only the geometry decides whether legs have to be re-fetched: renaming a place or diff --git a/client/src/store/slices/remoteEventHandler.ts b/client/src/store/slices/remoteEventHandler.ts index d7877ee61..e08b21e58 100644 --- a/client/src/store/slices/remoteEventHandler.ts +++ b/client/src/store/slices/remoteEventHandler.ts @@ -252,11 +252,17 @@ export const STATE_APPLIERS: Partial> } // Genuinely new — including a legitimate second assignment of a place - // already on this day (no temp version to reconcile). Append. + // already on this day (no temp version to reconcile). Seated where the server + // put it: a booked night lands at the front of its day, and the rows behind it + // were moved up one there. Appending it with that index would leave it tied + // with the old first row and drawn second until the next reload. + const ordered = existing.slice().sort((a, b) => (a.order_index ?? 0) - (b.order_index ?? 0)) + const at = Math.max(0, Math.min(incoming.order_index ?? ordered.length, ordered.length)) + ordered.splice(at, 0, incoming) return { assignments: { ...state.assignments, - [dayKey]: [...existing, incoming], + [dayKey]: ordered.map((a, i) => (a.order_index === i ? a : { ...a, order_index: i })), } } }, diff --git a/server/src/nest/accommodations/accommodations.service.ts b/server/src/nest/accommodations/accommodations.service.ts index b140feb9f..1d0eade62 100644 --- a/server/src/nest/accommodations/accommodations.service.ts +++ b/server/src/nest/accommodations/accommodations.service.ts @@ -164,9 +164,16 @@ export class AccommodationsService { 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, checkIn?: string | null): AccommodationMirror { - return this.remirrorStay(accommodationId, placeId, dayId, checkIn); + /** Carry a stay's own stop over to where the stay now is. `checkInChanged` says the + * booking was given a new hour, which seats the night afresh (see remirrorStay). */ + moveStayStop( + accommodationId: number, + placeId: number | null, + dayId: number, + checkIn?: string | null, + opts: { checkInChanged?: boolean } = {}, + ): AccommodationMirror { + return this.remirrorStay(accommodationId, placeId, dayId, checkIn, opts); } /** Take back the stops of a stay that is being deleted elsewhere. */ @@ -277,17 +284,16 @@ export class AccommodationsService { /** * 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. + * First. The hotel is where the day is based: the traveller checks in and the stops + * they placed without an hour follow from there. Only a stop with a clock of its + * own can stand ahead of it, and only when that clock is at or before the check-in: + * a stop pinned to eight with a check-in at ten puts the night second, one pinned + * to the afternoon does not. It used to go last unless something pinned later + * pulled it forward, and a night booked for ten in the morning sat behind a whole + * day of unpinned stops, reached at a quarter past twelve. */ - private positionForCheckIn(dayId: number, checkIn: string | null | undefined, excludeId?: number): number | undefined { - if (!checkIn) return undefined; + private positionForCheckIn(dayId: number, checkIn: string | null | undefined, excludeId?: number): number { + if (!checkIn) return 0; // excludeId leaves the row being re-seated out of the chain it is measured // against. Without it a night parked at the end of the day can find itself. const rows = this.db.all<{ order_index: number; at: string | null }>(` @@ -295,18 +301,20 @@ export class AccommodationsService { FROM day_assignments da JOIN places p ON p.id = da.place_id WHERE da.day_id = ? AND da.id != ? ORDER BY da.order_index `, dayId, excludeId ?? -1); - const later = rows.find(row => row.at !== null && row.at > checkIn); - return later?.order_index; + let seat = 0; + for (const row of rows) if (row.at !== null && row.at <= checkIn) seat = row.order_index + 1; + return seat; } /** - * Whether the night already sits where its check-in says it should. + * Whether the night sits somewhere its check-in allows, on an edit that did not + * touch the check-in. * * 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. + * without an hour are passed over in both directions: the night may have been + * dragged past them on purpose, and a change of notes is no reason to undo that. */ private seatedByCheckIn(dayId: number, ownId: number, checkIn: string | null | undefined): boolean { if (!checkIn) return true; @@ -493,7 +501,7 @@ export class AccommodationsService { this.db.run('UPDATE day_assignments SET day_id = ?, place_id = ?, order_index = ? WHERE id = ?', dayId, placeId, end, stop.id); const seat = this.positionForCheckIn(dayId, checkIn, stop.id); - if (seat !== undefined && seat < end) { + if (seat < end) { this.db.run( 'UPDATE day_assignments SET order_index = order_index + 1 WHERE day_id = ? AND order_index >= ? AND id != ?', dayId, seat, stop.id, @@ -519,9 +527,8 @@ export class AccommodationsService { if (this.db.get('SELECT id FROM day_assignments WHERE day_id = ? AND place_id = ?', dayId, placeId)) return mirror; const before = this.stopOrders([dayId]); - // 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. + // Through AssignmentsService, seated where the check-in says (see + // positionForCheckIn), with everything behind it moved up one. // // The booking id goes in with the INSERT, not as an UPDATE afterwards: what this // returns is the row the answer hands the client, and stamping the id on later @@ -590,14 +597,24 @@ export class AccommodationsService { * * Runs inside the caller's transaction. */ - private remirrorStay(accommodationId: number, placeId: number | null, dayId: number, checkIn?: string | null): AccommodationMirror { + private remirrorStay( + accommodationId: number, + placeId: number | null, + dayId: number, + checkIn?: string | null, + opts: { checkInChanged?: boolean } = {}, + ): 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) { - // 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(); + // Same place, same day. A check-in given a new hour seats the night afresh, the + // way booking it with that hour would have; any other edit leaves a stop where + // it is unless the clocks around it say it is in the wrong place, so a night the + // traveller dragged somewhere stays there through a change of notes. + const settled = opts.checkInChanged + ? this.positionForCheckIn(dayId, checkIn, own[0].id) === own[0].order_index + : this.seatedByCheckIn(dayId, own[0].id, checkIn); + if (settled) return noMirror(); } const mirror = noMirror(); @@ -689,7 +706,9 @@ 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, newCheckIn); + return this.remirrorStay(Number(id), newPlaceId, newStartDayId, newCheckIn, { + checkInChanged: fields.check_in !== undefined && (fields.check_in || null) !== (existing.check_in || null), + }); }); // Sync check-in/out/confirmation to every linked reservation. The booking form diff --git a/server/src/nest/reservations/reservations.service.ts b/server/src/nest/reservations/reservations.service.ts index 5d190e8ba..eb4ae18a1 100644 --- a/server/src/nest/reservations/reservations.service.ts +++ b/server/src/nest/reservations/reservations.service.ts @@ -846,6 +846,7 @@ export class ReservationsService { if (start_day_id && end_day_id) { this.requireResolvableStay(create_accommodation); if (resolvedAccId) { + const prior = this.db.get<{ check_in: string | null }>('SELECT check_in FROM day_accommodations WHERE id = ?', resolvedAccId); this.db.run( 'UPDATE day_accommodations SET place_id = ?, start_day_id = ?, end_day_id = ?, check_in = ?, check_out = ?, confirmation = ? WHERE id = ?', accPlaceId || null, start_day_id, end_day_id, check_in || null, check_out || null, accConf || confirmation_number || null, resolvedAccId @@ -853,7 +854,9 @@ 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, check_in); + stayMirror = this.accommodations.moveStayStop(resolvedAccId, accPlaceId || null, start_day_id, check_in, { + checkInChanged: (check_in || null) !== (prior?.check_in ?? null), + }); } 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 (?, ?, ?, ?, ?, ?, ?)', diff --git a/server/src/nest/roadtrip/roadtrip-plan.service.ts b/server/src/nest/roadtrip/roadtrip-plan.service.ts index 91a8a5f9b..7e99ceb05 100644 --- a/server/src/nest/roadtrip/roadtrip-plan.service.ts +++ b/server/src/nest/roadtrip/roadtrip-plan.service.ts @@ -22,6 +22,7 @@ import { type RoutedLeg, type SnappedWaypoint, type RouteAvoidClass, + standsAsDay, } from '@trek/shared/roadtrip'; interface StoredDay { @@ -32,6 +33,8 @@ interface StoredDay { default_transport_mode: string | null; } interface VisitRow { + /** The booking this stop stands for on its check-in day, when there is one. */ + stay_id: number | null; check_in: string | null; /** The drive does not read these two (#2357): they are the booking as get_roadtrip_context reports it. */ check_out: string | null; @@ -75,7 +78,7 @@ export class RoadtripPlanService { `SELECT a.id, a.day_id, a.place_id, p.name, p.lat, p.lng, COALESCE(a.assignment_time, p.place_time) AS time, COALESCE(a.assignment_end_time, p.end_time) AS end_time, p.duration_minutes, a.end_day, - a.leg_transport_mode, a.incoming_leg_transport_mode, p.stop_type, p.fill_percent, stay.check_in, stay.check_out, checkout.day_number AS checkout_day + a.leg_transport_mode, a.incoming_leg_transport_mode, p.stop_type, p.fill_percent, stay.id AS stay_id, stay.check_in, stay.check_out, checkout.day_number AS checkout_day FROM day_assignments a JOIN days d ON d.id = a.day_id JOIN places p ON p.id = a.place_id LEFT JOIN day_accommodations stay ON stay.id = (SELECT id FROM day_accommodations WHERE place_id = p.id AND start_day_id = d.id ORDER BY id LIMIT 1) LEFT JOIN days checkout ON checkout.id = stay.end_day_id @@ -123,6 +126,7 @@ export class RoadtripPlanService { // planner makes in the browser (useRoadtripRoutes). leaveAt: v.end_time, checkInTime: v.check_in, + night: v.stay_id !== null, dwellMinutes: v.duration_minutes, endDay: v.end_day === 1, legMode: v.leg_transport_mode, @@ -245,8 +249,8 @@ export class RoadtripPlanService { preferences.roadtrip_range_km, ) || null; const calculated = assembleRoadtrip({ - plan: plan.filter((d) => d.stops.length > 1), - quietDays: plan.filter((d) => d.stops.length < 2), + plan: plan.filter((d) => standsAsDay(d.stops)), + quietDays: plan.filter((d) => !standsAsDay(d.stops)), window, allLegs, snapByDay, diff --git a/server/tests/unit/nest/accommodations.service.test.ts b/server/tests/unit/nest/accommodations.service.test.ts index 0fdd0e26c..c5223d820 100644 --- a/server/tests/unit/nest/accommodations.service.test.ts +++ b/server/tests/unit/nest/accommodations.service.test.ts @@ -615,7 +615,10 @@ describe('the day stop a booking implies', () => { expect(mirror.created.accommodation_id).toBe(accommodation.id); }); - it('ACC-022 the stop lands at the end of the day, where you arrive at a hotel', () => { + it('ACC-022 the stop lands first: the hotel is where the day is based', () => { + // It used to go last, and a night booked for the morning sat behind a whole day + // of unpinned stops. The stops the traveller placed without an hour follow the + // hotel; only a clock of their own can put one ahead of it (ACC-022c). const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); const day = createDay(testDb, trip.id); @@ -625,13 +628,12 @@ describe('the day stop a booking implies', () => { book(trip.id, hotel.id, day.id, day.id); - expect(stopsOn(day.id).map(a => a.place_id)).toEqual([museum.id, hotel.id]); + expect(stopsOn(day.id).map(a => a.place_id)).toEqual([hotel.id, museum.id]); + expect(stopsOn(day.id).map(a => a.order_index)).toEqual([0, 1]); }); 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. + // A stop pinned to the afternoon cannot stand ahead of a check-in at eleven. const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); const day = createDay(testDb, trip.id); @@ -645,7 +647,7 @@ describe('the day stop a booking implies', () => { 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', () => { + it('ACC-022c a stop pinned to an earlier hour stays ahead of the check-in', () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); const day = createDay(testDb, trip.id); @@ -659,18 +661,67 @@ describe('the day stop a booking implies', () => { 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. + it('ACC-022d a stop without an hour of its own follows the night, whatever the check-in', () => { + // A stop with no clock is planned from the hotel, not the other way round. This + // is the day from Discord: two unpinned stops and a night booked for ten in the + // morning, which the drive used to reach at a quarter past twelve. 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); + const fuel = createPlace(testDb, trip.id, { name: 'Aral' }); + const farm = createPlace(testDb, trip.id, { name: 'Karls Erdbeerhof' }); + const hotel = createPlace(testDb, trip.id, { name: 'Rostock' }); + createDayAssignment(testDb, day.id, fuel.id); + createDayAssignment(testDb, day.id, farm.id); - svc.createAccommodation(trip.id, { place_id: hotel.id, start_day_id: day.id, end_day_id: day.id, check_in: '11:00' }); + svc.createAccommodation(trip.id, { place_id: hotel.id, start_day_id: day.id, end_day_id: day.id, check_in: '10:00' }); - expect(stopsOn(day.id).map(a => a.place_id)).toEqual([untimed.id, hotel.id]); + expect(stopsOn(day.id).map(a => a.place_id)).toEqual([hotel.id, fuel.id, farm.id]); + }); + + it('ACC-022f the night goes behind the last stop whose own hour is at or before the check-in', () => { + // Pinned eight, unpinned, pinned two: a check-in at ten sits behind the eight and + // ahead of everything else, the unpinned stop included. + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id); + const [early, loose, late, hotel] = ['Aral', 'Hafen', 'Museum', 'Rostock'].map(name => createPlace(testDb, trip.id, { name })); + const first = createDayAssignment(testDb, day.id, early.id); + createDayAssignment(testDb, day.id, loose.id); + const third = createDayAssignment(testDb, day.id, late.id); + testDb.prepare("UPDATE day_assignments SET assignment_time = '08:00' WHERE id = ?").run(first.id); + testDb.prepare("UPDATE day_assignments SET assignment_time = '14:00' WHERE id = ?").run(third.id); + + svc.createAccommodation(trip.id, { place_id: hotel.id, start_day_id: day.id, end_day_id: day.id, check_in: '10:00' }); + + expect(stopsOn(day.id).map(a => a.place_id)).toEqual([early.id, hotel.id, loose.id, late.id]); + }); + + it('ACC-022g a new check-in seats the night afresh, a change of notes leaves a dragged night alone', () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id); + const [harbour, market, hotel] = ['Hafen', 'Markt', 'Rostock'].map(name => createPlace(testDb, trip.id, { name })); + createDayAssignment(testDb, day.id, harbour.id); + createDayAssignment(testDb, day.id, market.id); + const { accommodation } = svc.createAccommodation(trip.id, { place_id: hotel.id, start_day_id: day.id, end_day_id: day.id }) as any; + expect(stopsOn(day.id).map(a => a.place_id)).toEqual([hotel.id, harbour.id, market.id]); + + // Dragged to the end of the day by hand. + const own = stopsOn(day.id).find(a => a.place_id === hotel.id)!; + testDb.prepare('UPDATE day_assignments SET order_index = order_index - 1 WHERE day_id = ? AND order_index > ?').run(day.id, own.order_index); + testDb.prepare('UPDATE day_assignments SET order_index = 2 WHERE id = ?').run(own.id); + expect(stopsOn(day.id).map(a => a.place_id)).toEqual([harbour.id, market.id, hotel.id]); + + let existing = svc.getAccommodation(accommodation.id, trip.id)!; + const quiet = svc.updateAccommodation(accommodation.id, existing, { notes: 'late arrival' }) as any; + expect(quiet.mirror.moved).toBeNull(); + expect(stopsOn(day.id).map(a => a.place_id)).toEqual([harbour.id, market.id, hotel.id]); + + existing = svc.getAccommodation(accommodation.id, trip.id)!; + const reseated = svc.updateAccommodation(accommodation.id, existing, { check_in: '10:00' }) as any; + expect(reseated.mirror.moved).toMatchObject({ oldDayId: day.id, assignment: { id: own.id } }); + expect(stopsOn(day.id).map(a => a.place_id)).toEqual([hotel.id, harbour.id, market.id]); }); it('ACC-022e moving the check-in later re-seats the night', () => { @@ -887,7 +938,10 @@ describe('the day stop a booking implies', () => { createDayAssignment(testDb, day.id, dinner.id); const { accommodation } = book(trip.id, hotel.id, day.id, day.id); const stop = stopsOn(day.id).find((row: { place_id: number }) => row.place_id === hotel.id)!; - expect(stop.order_index).toBe(1); + // The backfill's placement, written by hand: the night behind the dinner. + testDb.prepare('UPDATE day_assignments SET order_index = 0 WHERE day_id = ? AND place_id = ?').run(day.id, dinner.id); + testDb.prepare('UPDATE day_assignments SET order_index = 1 WHERE id = ?').run(stop.id); + expect(stopsOn(day.id).map((row: { place_id: number }) => row.place_id)).toEqual([dinner.id, hotel.id]); const existing = svc.getAccommodation(accommodation.id, trip.id)!; const { mirror } = svc.updateAccommodation(accommodation.id, existing, { check_in: '15:00' }) as any; @@ -1051,13 +1105,13 @@ describe('the drawn roads a booking moves', () => { const pin = (assignmentId: number, time: string) => testDb.prepare('UPDATE day_assignments SET assignment_time = ? WHERE id = ?').run(time, assignmentId); - /** A(0), B(1) pinned to noon, C(2): the day a check-in at eleven lands in the middle of. */ + /** A(0) pinned to nine, B(1) pinned to noon, C(2): the day a check-in at eleven lands in the middle of. */ function afternoonDay() { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); const day = createDay(testDb, trip.id); const [a, b, c, hotel] = ['Harbour', 'Mercure', 'Museum', 'Billstedt'].map(name => createPlace(testDb, trip.id, { name })); - createDayAssignment(testDb, day.id, a.id); + pin(createDayAssignment(testDb, day.id, a.id).id, '09:00'); pin(createDayAssignment(testDb, day.id, b.id).id, '12:00'); createDayAssignment(testDb, day.id, c.id); return { trip, day, a, b, c, hotel }; @@ -1091,20 +1145,22 @@ describe('the drawn roads a booking moves', () => { }]); }); - it('ACC-034b a night appended at the end of the day moves no road and reports none', () => { + it('ACC-034b a night seated behind every drawn road moves none and reports none', () => { const { trip, day, a, b, c, hotel } = afternoonDay(); const afterA = addVia(day.id, 0); const afterB = addVia(day.id, 1); - const { mirror } = svc.createAccommodation(trip.id, { place_id: hotel.id, start_day_id: day.id, end_day_id: day.id }) as any; + const { mirror } = svc.createAccommodation(trip.id, { place_id: hotel.id, start_day_id: day.id, end_day_id: day.id, check_in: '18:00' }) as any; - expect(stopsOn(day.id)).toEqual([a.id, b.id, c.id, hotel.id]); + expect(stopsOn(day.id)).toEqual([a.id, b.id, hotel.id, c.id]); expect(viaAnchors(day.id)).toEqual([{ id: afterA, after_order_index: 0 }, { id: afterB, after_order_index: 1 }]); expect(mirror.vias).toBeUndefined(); }); it('ACC-034c re-seating the night by a later check-in re-pins the roads around it', () => { const { trip, day, a, b, c, hotel } = afternoonDay(); + // C pinned to the afternoon as well, so an evening check-in puts the night last. + testDb.prepare("UPDATE day_assignments SET assignment_time = '15:00' WHERE day_id = ? AND place_id = ?").run(day.id, c.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)).toEqual([a.id, hotel.id, b.id, c.id]); const afterA = addVia(day.id, 0); @@ -1134,7 +1190,7 @@ describe('the drawn roads a booking moves', () => { const trip = createTrip(testDb, user.id); const day = createDay(testDb, trip.id); const [a, b, hotel] = ['Harbour', 'Mercure', 'Billstedt'].map(name => createPlace(testDb, trip.id, { name })); - createDayAssignment(testDb, day.id, a.id); + pin(createDayAssignment(testDb, day.id, a.id).id, '09:00'); pin(createDayAssignment(testDb, day.id, b.id).id, '12:00'); 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)).toEqual([a.id, hotel.id, b.id]); @@ -1168,7 +1224,7 @@ describe('the drawn roads a booking moves', () => { const day1 = createDay(testDb, trip.id); const day2 = createDay(testDb, trip.id); const [a, b, c, hotel] = ['Harbour', 'Mercure', 'Museum', 'Billstedt'].map(name => createPlace(testDb, trip.id, { name })); - createDayAssignment(testDb, day1.id, a.id); + pin(createDayAssignment(testDb, day1.id, a.id).id, '09:00'); pin(createDayAssignment(testDb, day1.id, b.id).id, '12:00'); pin(createDayAssignment(testDb, day2.id, c.id).id, '14:00'); const { accommodation } = svc.createAccommodation(trip.id, { place_id: hotel.id, start_day_id: day1.id, end_day_id: day1.id, check_in: '11:00' }) as any; diff --git a/shared/src/roadtrip/assemble.spec.ts b/shared/src/roadtrip/assemble.spec.ts index cd0bb5de2..58b12b81e 100644 --- a/shared/src/roadtrip/assemble.spec.ts +++ b/shared/src/roadtrip/assemble.spec.ts @@ -294,3 +294,47 @@ describe('assembleRoadtrip leave times', () => { expect(schedule.warnings).toContainEqual({ index: 2, code: 'late', minutes: 30 }); }); }); + +describe('assembleRoadtrip booked nights', () => { + // 90 km at the 40 seconds a kilometre `leg` drives is an hour. + const hour = 90; + + it('ROADTRIP-ASSEMBLE-012: a day that is only its booked night stands among the days', () => { + // The arrival day of a trip: nothing but the hotel, entered under Days with its + // check-in. It used to be filed under the quiet days at the foot of the rail, which + // reads as the hotel missing from the road trip altogether. + const routes = assemble([stop({ ownerIndex: 0, night: true, checkInTime: '15:00', stopType: 'hotel' })], []); + + expect(routes.quietDays).toEqual([]); + expect(routes.days).toHaveLength(1); + const day = routes.days[0]!; + expect(day.legs).toEqual([]); + expect(day.distance).toBe(0); + expect(day.schedule.entries[0]!).toMatchObject({ arrival: '15:00', anchored: true }); + }); + + it('ROADTRIP-ASSEMBLE-013: a lone stop that is no night stays a quiet day', () => { + const routes = assemble([stop({ ownerIndex: 0 })], []); + + expect(routes.days).toEqual([]); + expect(routes.quietDays).toHaveLength(1); + }); + + it('ROADTRIP-ASSEMBLE-014: the check-in holds the night and the day lines up behind it', () => { + // The day from Discord: two stops without an hour and the night booked for ten, + // which the drive used to reach at a quarter past twelve. Seated first by the + // server now, and held at ten by the schedule; the stops follow from there. + const stops = [ + stop({ ownerIndex: 0, night: true, checkInTime: '10:00', dwellMinutes: 60, stopType: 'hotel' }), + stop({ ownerIndex: 1, dwellMinutes: 30 }), + stop({ ownerIndex: 2, dwellMinutes: 60 }), + ]; + + const routes = assemble(stops, [hour, hour]); + + const { schedule } = routes.days[0]!; + expect(schedule.entries.map((e) => e.arrival)).toEqual(['10:00', '12:00', '13:30']); + expect(schedule.entries[0]!.anchored).toBe(true); + expect(schedule.warnings).toEqual([]); + }); +}); diff --git a/shared/src/roadtrip/assemble.ts b/shared/src/roadtrip/assemble.ts index 58bab1de2..1f41abca1 100644 --- a/shared/src/roadtrip/assemble.ts +++ b/shared/src/roadtrip/assemble.ts @@ -21,6 +21,7 @@ import { refuelsRange, isServiceStopType, scheduleStopOf, + standsAsDay, type DryPoint, type DriveLimits, type VehicleKind, @@ -278,7 +279,7 @@ export function assembleRoadtrip({ dayWarning: drive.day, }); } - const drives = out.filter((d) => d.stops.length > 1 || !!d.spills?.length || d.stops.some((s) => s.automaticNight)); + const drives = out.filter((d) => standsAsDay(d.stops) || !!d.spills?.length || d.stops.some((s) => s.automaticNight)); const originalStops = [...plan, ...quietDays].sort((a, b) => a.dayNumber - b.dayNumber).flatMap((day) => day.stops); const boundaryPath = originalStops.slice(0, -1).flatMap((from, position) => { const to = originalStops[position + 1]!; @@ -317,7 +318,7 @@ export function assembleRoadtrip({ ), quietDays: out - .filter((d) => d.stops.length < 2 && !d.spills?.length && !d.stops.some((s) => s.automaticNight)) + .filter((d) => !standsAsDay(d.stops) && !d.spills?.length && !d.stops.some((s) => s.automaticNight)) .map((d) => ({ dayId: d.dayId, dayNumber: d.dayNumber, date: d.date, title: d.title, stops: d.stops })), loading, }; diff --git a/shared/src/roadtrip/dayWindow.spec.ts b/shared/src/roadtrip/dayWindow.spec.ts index 2bc82739d..365b01823 100644 --- a/shared/src/roadtrip/dayWindow.spec.ts +++ b/shared/src/roadtrip/dayWindow.spec.ts @@ -394,32 +394,51 @@ describe('daily travel window', () => { }); }); -it('waits for a check-in the drive overshoots rather than rushing it', () => { +it('holds a check-in the drive reaches within the minute a rounded leg can add', () => { const stops = [ stop(1, { time: '07:00', dwellMinutes: 60 }), stop(2, { checkInTime: '09:11' }), stop(3), ]; - const plan = calculate(stops, [72.5, 29]); + const plan = calculate(stops, [71.5, 29]); expect(plan.issue).toBeNull(); - // Arrives two minutes past the hour it may check in from, and is left there rather - // than pulled back to the round number. - expect(plan.chains[0]!.schedule.entries[1]!.arrival).toBe('09:13'); + // Half a minute over is the rounding of the drive, not being late: the night keeps + // its check-in, the same grace a pinned stop gets. + expect(plan.chains[0]!.schedule.entries[1]!.arrival).toBe('09:11'); const stops0 = plan.chains[0]!.stops; expect(stops0.at(-1)!.placeId).toBe(3); - // And carries straight on from there: nothing holds the traveller at the stop. - expect(plan.chains[0]!.schedule.entries.at(-1)!.arrival).toBe('09:42'); + expect(plan.chains[0]!.schedule.entries.at(-1)!.arrival).toBe('09:40'); }); -it('waits for check-in without treating it as a fixed appointment', () => { +it('does not rush a check-in the drive overshoots, and does not quietly move it either', () => { + // Two minutes past the check-in is past it. A pinned time out of reach is a + // conflict here, and a check-in is one of those now. + const stops = [ + stop(1, { time: '07:00', dwellMinutes: 60 }), + stop(2, { checkInTime: '09:11' }), + stop(3), + ]; + expect(calculate(stops, [72.5, 29]).issue).toBe('conflict'); +}); + +it('waits for a check-in the drive reaches early', () => { const plan = calculate([stop(1), stop(2, { checkInTime: '15:00' }), stop(3)], [60, 29]); expect(plan.issue).toBeNull(); - // The check-in is a door opening, not an appointment: the drive waits for it and then - // carries straight on, rather than the day being rebuilt around it. + // The night is held to its check-in the way a pinned stop is held to its time: the + // drive waits for it and carries on from there. expect(plan.chains[0]!.schedule.entries[1]!.arrival).toBe('15:00'); + expect(plan.chains[0]!.schedule.entries[1]!.anchored).toBe(true); expect(plan.chains[0]!.schedule.entries.at(-1)!.arrival).toBe('15:29'); }); +it('treats a check-in the drive cannot make as the conflict a missed pin is', () => { + // A night booked for ten with two hours of driving pinned ahead of it. The window + // planner does not invent a plan around a time it cannot keep; it hands back to the + // plain schedule, which keeps the ten and says how late the drive is. + const plan = calculate([stop(1, { time: '09:00' }), stop(2, { checkInTime: '10:00' })], [120]); + expect(plan.issue).toBe('conflict'); +}); + describe('a time set to leave a stop, with daily travel times', () => { it('spends the hours until then rather than the stay', () => { const plan = calculate( diff --git a/shared/src/roadtrip/dayWindow.ts b/shared/src/roadtrip/dayWindow.ts index 4cc22be37..27d8d1125 100644 --- a/shared/src/roadtrip/dayWindow.ts +++ b/shared/src/roadtrip/dayWindow.ts @@ -207,7 +207,10 @@ export function planDayWindow( let previous: RoadtripStop | undefined; for (let i = 0; i < stops.length; i++) { const { stop, day } = stops[i]!; - const pin = parseClock(stop.time); + // A booked night's check-in pins the stop the way its own time would: the day is + // built to be there by then, and a drive that cannot make it is the same conflict + // a pinned stop out of reach is. + const pin = parseClock(stop.time) ?? parseClock(stop.checkInTime); const leave = parseClock(stop.leaveAt); // When the drive into this stop set out, on this day's clock: for one a night broke // up, the morning it went on. @@ -290,7 +293,6 @@ export function planDayWindow( const target = targets.get(number); if (target !== undefined && target < position) return failed('conflict'); if (pin !== null) clock = pin; - if (pin === null && number === day.dayNumber) clock = Math.max(clock, parseClock(stop.checkInTime) ?? 0); append(stop, clock); previous = stop; diff --git a/shared/src/roadtrip/planning-types.ts b/shared/src/roadtrip/planning-types.ts index dcf41bec9..18d143ab9 100644 --- a/shared/src/roadtrip/planning-types.ts +++ b/shared/src/roadtrip/planning-types.ts @@ -25,6 +25,8 @@ export interface RoadtripStop { dwellMinutes: number | null; checkInTime?: string | null; + /** A booked night's stop on its check-in day, whether or not the booking names an hour. */ + night?: boolean; endDay?: boolean; legMode: string | null; diff --git a/shared/src/roadtrip/roadtripModel.spec.ts b/shared/src/roadtrip/roadtripModel.spec.ts index d806b5eb5..14d5931e5 100644 --- a/shared/src/roadtrip/roadtripModel.spec.ts +++ b/shared/src/roadtrip/roadtripModel.spec.ts @@ -1064,7 +1064,6 @@ describe('scheduleStopOf', () => { it('turns a leave time into a departure', () => { expect(scheduleStopOf({ time: '10:00', checkInTime: null, dwellMinutes: 60, leaveAt: '14:00' })).toEqual({ anchor: '10:00', - earliest: null, dwellMinutes: 60, departureAt: 840, }); @@ -1073,11 +1072,21 @@ describe('scheduleStopOf', () => { it('leaves the departure out when there is no usable leave time', () => { expect(scheduleStopOf({ time: null, dwellMinutes: 30 })).toEqual({ anchor: null, - earliest: null, dwellMinutes: 30, }); expect(scheduleStopOf({ time: null, dwellMinutes: 30, leaveAt: 'soon' })).not.toHaveProperty('departureAt'); }); + + it('anchors a booked night on its check-in', () => { + expect(scheduleStopOf({ time: null, checkInTime: '10:00', dwellMinutes: 60 })).toEqual({ + anchor: '10:00', + dwellMinutes: 60, + }); + }); + + it('lets a time pinned on the stop itself win over the check-in', () => { + expect(scheduleStopOf({ time: '16:00', checkInTime: '15:00', dwellMinutes: 60 }).anchor).toBe('16:00'); + }); }); @@ -1104,19 +1113,19 @@ describe('hasChosenArrival', () => { }); }); -describe('a check-in is a door opening, not an appointment', () => { - // The hour a room becomes available. Reaching it later is reaching it; only a time - // somebody pinned to a stop can be missed. Read as an anchor it did the opposite: - // arriving at 16:14 was reported as 5 h 14 late against an 11:00 check-in, and the - // hotel was pushed onto the next day because 11:00 reads as earlier than the 15:00 - // before it. +describe('a check-in holds a booked night the way a pinned time does', () => { + // The hour the traveller said they are at the hotel. It used to be a floor, waited + // for when the drive got there first and silent otherwise, and once anything before + // it set the clock it held nothing: a night booked for ten in the morning was + // reported reached at a quarter past twelve, with the rest of the day lined up + // behind that. The check-in goes in as the anchor now (see scheduleStopOf), so + // these run on the anchor the way the rail does. it('waits for it when the drive gets there first', () => { const schedule = computeSchedule( - [{ anchor: '09:00', dwellMinutes: 0 }, { anchor: null, earliest: '15:00', dwellMinutes: 0 }], + [{ anchor: '09:00', dwellMinutes: 0 }, { anchor: '15:00', dwellMinutes: 0 }], [3600], ); expect(schedule.entries[1]!.arrival).toBe('15:00'); - // Waiting for a door is waiting for a time somebody named, so it is printed as one. expect(schedule.entries[1]!.anchored).toBe(true); expect(schedule.warnings).toEqual([]); }); @@ -1126,7 +1135,7 @@ describe('a check-in is a door opening, not an appointment', () => { // Read only as a floor, the chain worked the hotel backwards out of the afternoon // and put it at 13:46: true enough as arithmetic, and nothing anybody asked for. const schedule = computeSchedule( - [{ anchor: null, earliest: '10:00', dwellMinutes: 60 }, { anchor: '15:00', dwellMinutes: 60 }], + [{ anchor: '10:00', dwellMinutes: 60 }, { anchor: '15:00', dwellMinutes: 60 }], [14 * 60], ); expect(schedule.entries[0]!.arrival).toBe('10:00'); @@ -1135,18 +1144,21 @@ describe('a check-in is a door opening, not an appointment', () => { expect(schedule.warnings).toEqual([]); }); - it('is simply arrived at when the drive gets there later', () => { + it('keeps its hour and says how late the drive is when it gets there later', () => { + // Two stops, then the hotel booked for ten: the drive reaches it at 12:17. The + // night stays at ten and the day is lined up behind that, and the finding says + // by how much the drive misses it, the way it would for any pinned stop. const schedule = computeSchedule( - [{ anchor: '15:00', dwellMinutes: 60 }, { anchor: null, earliest: '11:00', dwellMinutes: 60 }], - [14 * 60], + [{ anchor: '08:00', dwellMinutes: 30 }, { anchor: null, dwellMinutes: 60 }, { anchor: '10:00', dwellMinutes: 60 }], + [89 * 60, 78 * 60], ); - expect(schedule.entries[1]!.arrival).toBe('16:14'); - // Nothing was decided here, so it reads as computed. - expect(schedule.entries[1]!.anchored).toBe(false); - // And nothing was missed. - expect(schedule.warnings).toEqual([]); + expect(schedule.entries[1]!.arrival).toBe('09:59'); + expect(schedule.entries[2]!.arrival).toBe('10:00'); + expect(schedule.entries[2]!.anchored).toBe(true); + expect(schedule.entries[2]!.departure).toBe('11:00'); + expect(schedule.warnings).toEqual([{ index: 2, code: 'late', minutes: 137 }]); // Still the same day, which is what the day split reads. - expect(schedule.entries[1]!.dayOffset).toBe(0); + expect(schedule.entries[2]!.dayOffset).toBe(0); }); it('still reports a pinned time that cannot be made', () => { diff --git a/shared/src/roadtrip/roadtripModel.ts b/shared/src/roadtrip/roadtripModel.ts index 8e7b65422..a70d360ab 100644 --- a/shared/src/roadtrip/roadtripModel.ts +++ b/shared/src/roadtrip/roadtripModel.ts @@ -42,12 +42,12 @@ export function formatDurationShort(seconds: number): string { * Whether this stop's arrival is a time somebody chose, rather than one the chain * worked out from the stop before it. * - * A check-in counts. Every schedule already anchors on `time ?? checkInTime`, so a - * booked night starts its day exactly like a pinned stop does; asking only about - * `time` here made the rail print that chosen hour in the grey it reserves for - * computed ones. One function because the answer was spelled out separately in each - * of the three schedulers, and two of them spelled it differently from the anchor - * they had just used. + * A check-in counts. Every schedule anchors on `time ?? checkInTime`, so a booked + * night holds its hour exactly like a pinned stop does; asking only about `time` + * here made the rail print that chosen hour in the grey it reserves for computed + * ones. One function because the answer was spelled out separately in each of the + * three schedulers, and two of them spelled it differently from the anchor they had + * just used. */ export function hasChosenArrival(stop: { time?: string | null; checkInTime?: string | null; automaticNight?: unknown }): boolean { if (stop.automaticNight) return false; @@ -83,16 +83,9 @@ export interface ScheduleStop { */ departureAt?: number; /** A time somebody fixed this stop to. The chain restarts from it, and arriving - * after it is being late. */ + * after it is being late. A booked night's check-in is one of these. */ anchor: string | null; - /** - * The earliest this stop can be entered, when something says so — a check-in is - * the hour a room becomes available, not an appointment. Arriving before it means - * waiting for it; arriving after it means arriving, with nothing to report. - */ - earliest?: string | null; - dwellMinutes: number | null; } @@ -222,6 +215,13 @@ export function leaveAfter( /** * The part of a road trip stop the schedule reads. * + * A booked night's check-in is the hour the traveller said they are at the hotel, so + * it holds the stop the way a pinned time does: the day is built around it, and a + * drive that gets there later is late. Read as a mere floor it held nothing once + * anything before it set the clock, and a night booked for ten in the morning was + * reported reached at a quarter past twelve, with the rest of the day lined up + * behind that. A time pinned on the stop itself still wins. + * * A visit's end time is when the drive leaves it. It is the traveller's own statement * about this visit, unlike the check-out that used to feed `departureAt` (the LATEST a * room has to be handed back, which is why #2357 took it out of the drive). @@ -231,8 +231,7 @@ export function scheduleStopOf( ): ScheduleStop { const leave = parseClock(stop.leaveAt); return { - anchor: stop.time ?? null, - earliest: stop.checkInTime ?? null, + anchor: stop.time ?? stop.checkInTime ?? null, dwellMinutes: stop.dwellMinutes, ...(leave === null ? {} : { departureAt: leave }), }; @@ -267,22 +266,7 @@ export function computeSchedule( for (let i = 0; i < stops.length; i++) { const stop = stops[i]!; const anchor = parseClock(stop.anchor); - const resolved = resolveArrival(anchor, cursor, dayOffset); - // A door that opens at eleven is not an appointment at eleven. Reaching the stop - // later than that is simply reaching it; only a time somebody pinned can be missed. - const opens = parseClock(stop.earliest ?? null); - // With nothing before it deciding the hour, the door IS the hour: a night booked - // to check in at ten starts the day at ten rather than being worked backwards out - // of whatever comes after it. Reached later, it is only a floor, and a floor below - // the arrival changes nothing. - const waited = anchor === null && opens !== null - ? resolved.arrival === null - ? opens + dayOffset * DAY_MINUTES - : opens + Math.round((resolved.arrival - opens) / DAY_MINUTES) * DAY_MINUTES - : null; - const held = waited !== null && (resolved.arrival === null || waited > resolved.arrival); - const arrival = held ? waited : resolved.arrival; - const lateBy = resolved.lateBy; + const { arrival, lateBy } = resolveArrival(anchor, cursor, dayOffset); if (lateBy !== null) warnings.push({ index: i, code: 'late', minutes: lateBy }); if (arrival === null) { @@ -302,9 +286,7 @@ export function computeSchedule( if (offset > dayOffset) dayOffset = offset; arrivals[i]! = arrival; - // Ink for a time somebody decided: a pinned one always, a check-in only where the - // drive actually had to wait for it. - anchored[i]! = anchor !== null || held; + anchored[i]! = anchor !== null; let departure = arrival + (stop.dwellMinutes ?? 0); if (stop.departureAt !== undefined) { @@ -385,6 +367,20 @@ export function computeSchedule( return { entries, warnings, endsAt }; } +/** + * Whether the rail lists this day among the days, rather than as a quiet placeholder + * asking for stops. + * + * Two stops make a drive. A booked night makes a day on its own: the day you arrive + * somewhere and go no further, or the one you check in at ten and set out from. It + * used to need company, and a hotel entered under Days for the arrival day was filed + * at the bottom of the rail under "only X so far", which reads as the hotel missing + * from the road trip altogether. + */ +export function standsAsDay(stops: readonly { night?: boolean }[]): boolean { + return stops.length > 1 || stops.some((stop) => stop.night === true); +} + export function splitIntoRuns(stops: T[], modeOfLeg: (from: T, to: T) => string): { stops: T[]; mode: string }[] { const runs: { stops: T[]; mode: string }[] = []; let current: T[] = [];