k1nq/TREK
1
0
Fork 0
mirror of https://github.com/mauriceboe/TREK.git synced 2026-10-18 21:16:47 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(roadtrip): keep a booked night out of the day list, and off the traveller's stops

Browse Source 
Three things the first pass got wrong.

A stop a booking puts on the day is for road trip mode, which needs the
hotel in the driving chain. The day planner already shows that booking as
its own overnight block, so the row there would be the same hotel twice.
It is hidden in Days whatever the service-stop switch says, and the switch
goes back to meaning what it says: the petrol stations the traveller added.

Cancelling a night from the road trip popup took the stop with it. That
control means "this is a pause now, not a night", and the stop sits
mid-drive where re-adding it would land it at the end of the day. The
booking now hands the stop over instead of deleting it.

Moving a booking whose stop belongs to the traveller put a second one on
the new day and left the first behind, which is the duplicate this change
exists to remove. Only a booking's own stop moves now. Stays booked before
this shipped get theirs from the migration, not from the next edit they
happen to receive.
This commit is contained in:
Maurice
2026-09-13 01:42:40 +02:00
parent 462a208fdf
commit edf0538260
7 changed files with 110 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files
client/src/api/client.ts
+1 -1
View File
@@ -1428,7 +1428,7 @@ export const accommodationsApi = {
list: (tripId: number | string) => apiClient.get(`/trips/${tripId}/accommodations`).then(r => r.data),
create: (tripId: number | string, data: AccommodationCreateRequest) => apiClient.post(`/trips/${tripId}/accommodations`, data).then(r => r.data),
update: (tripId: number | string, id: number, data: AccommodationUpdateRequest) => apiClient.put(`/trips/${tripId}/accommodations/${id}`, data).then(r => r.data),
delete: (tripId: number | string, id: number) => apiClient.delete(`/trips/${tripId}/accommodations/${id}`).then(r => r.data),
delete: (tripId: number | string, id: number, opts?: { keepStop?: boolean }) => apiClient.delete(`/trips/${tripId}/accommodations/${id}`, opts?.keepStop ? { params: { keepStop: 'true' } } : undefined).then(r => r.data),
}
export const dayNotesApi = {
client/src/pages/tripPlanner/useTripPlanner.ts
+26 -5
View File
@@ -55,9 +55,10 @@ import { usePlannerHistory } from '../../hooks/usePlannerHistory'
import { useAirtrailConnection } from '../../hooks/useAirtrailConnection'
import { useIsTouch } from '../../hooks/useIsTouch'
import { usePluginStore } from '../../store/pluginStore'
import type { Accommodation, TripMember, Day, Place, Reservation } from '../../types'
import type { Accommodation, Assignment, TripMember, Day, Place, Reservation } from '../../types'
import { OFM_POSITRON, DEFAULT_MAP_LAT, DEFAULT_MAP_LNG, DEFAULT_MAP_ZOOM } from '../../constants/mapDefaults'
import { useTileUrl } from '../../hooks/useTileUrl'
import { applyStayStops } from '../../store/stayStops'
import { resolvePoolAssignmentId } from './tripPlannerModel'
import { isDeepLinkableTripTab, TRIP_TAB_LABEL_KEYS } from '../../constants/tripTabs'
import { isRoutableReservation } from '../../utils/reservationRoutes'
@@ -128,9 +129,27 @@ export function useTripPlanner() {
// someone planning a road trip stays in it across reloads without it leaking into
// their next, non-driving trip.
const [roadtripMode, setRoadtripMode] = useState<boolean>(() => sessionStorage.getItem(`trip-roadtrip-${tripId}`) === '1')
const assignments = useMemo(() => roadtripMode || roadtripSettings.roadtrip_service_stops_in_days !== false ? storedAssignments : Object.fromEntries(
Object.entries(storedAssignments).map(([dayId, visits]) => [dayId, visits.filter(visit => !isServiceStopType(visit.place?.stop_type))]),
), [roadtripMode, roadtripSettings.roadtrip_service_stops_in_days, storedAssignments])
// Two reasons a stop can be road-trip-only, and they are not the same reason.
//
// The switch is the traveller's: it hides the petrol stations and rest areas
// they added along the drive from a day list they want to read as a plan.
//
// A stop a lodging booking put there is hidden whatever the switch says,
// because the day already shows that booking as its own overnight block and
// the row would be the same hotel a second time. Road trip mode wants it: the
// drive has to end somewhere, and that somewhere is where you sleep.
const assignments = useMemo(() => {
if (roadtripMode) return storedAssignments
const hideServiceStops = roadtripSettings.roadtrip_service_stops_in_days === false
const hidden = (visit: Assignment) => visit.accommodation_id != null
|| (hideServiceStops && isServiceStopType(visit.place?.stop_type))
// Same object back when nothing is hidden, so a trip without bookings does not
// rebuild every day list on each render of this hook.
if (!Object.values(storedAssignments).some(visits => visits.some(hidden))) return storedAssignments
return Object.fromEntries(
Object.entries(storedAssignments).map(([dayId, visits]) => [dayId, visits.filter(v => !hidden(v))]),
)
}, [roadtripMode, roadtripSettings.roadtrip_service_stops_in_days, storedAssignments])
const places = useMemo(() => roadtripMode || roadtripSettings.roadtrip_service_stops_in_days !== false ? allPlaces : allPlaces.filter(place => !isServiceStopType(place.stop_type)), [roadtripMode, roadtripSettings.roadtrip_service_stops_in_days, allPlaces])
const toggleRoadtripMode = useCallback(() => {
setRoadtripMode(prev => {
@@ -937,7 +956,9 @@ export function useTripPlanner() {
if (stopDraft.editing) {
await tripActions.updatePlace(tripId, stopDraft.editing.placeId, { stop_type: stopType, duration_minutes: dwellMinutes })
if (stopDraft.editing.accommodationId) {
await accommodationsApi.delete(tripId, stopDraft.editing.accommodationId)
// The night is what was switched off, not the stop: it stays where it is
// in the drive and becomes an ordinary pause.
applyStayStops(await accommodationsApi.delete(tripId, stopDraft.editing.accommodationId, { keepStop: true }))
await loadAccommodations()
}
updateRouteForDay(dayId)
server/src/db/migrations.ts
+43 -3
View File
@@ -4735,13 +4735,53 @@ function runMigrations(db: Database.Database): void {
// 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.
// Every row that already exists stays NULL: those were planned by hand, and a
// booking must not start claiming ownership of them. The step below adds the
// missing stops instead, which is a different thing from claiming old ones.
() => {
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');
},
/*
* Give every stay booked before this release the day stop it would get today.
*
* Road trip mode builds its drive out of day_assignments alone, so a hotel
* booked in Days mode was invisible there and the traveller had to add the
* same place a second time by hand. New bookings get the stop as they are
* written; without this step the fix would only ever apply to trips planned
* after the upgrade, and the trips people already have would stay broken.
*
* Skipped on purpose: a stay whose place is gone (place_id is ON DELETE SET
* NULL, and the booking form writes stays that never had one), and a place the
* traveller already planned for that day, whose row stays theirs and unmarked.
* Re-runnable: the same NOT EXISTS decides both times.
*/
() => {
const stays = db.prepare(`
SELECT a.id, a.place_id, a.start_day_id
FROM day_accommodations a
WHERE a.place_id IS NOT NULL
AND NOT EXISTS (
SELECT 1 FROM day_assignments da
WHERE da.day_id = a.start_day_id AND da.place_id = a.place_id
)
ORDER BY a.id
`).all() as Array<{ id: number; place_id: number; start_day_id: number }>;
const insert = db.prepare(
`INSERT INTO day_assignments (day_id, place_id, order_index, accommodation_id)
VALUES (?, ?, COALESCE((SELECT MAX(order_index) + 1 FROM day_assignments WHERE day_id = ?), 0), ?)`
);
// Arriving somewhere is what the day was for, so the stop goes last, the same
// position a stay booked today lands in.
const stamp = db.prepare("UPDATE places SET stop_type = 'hotel' WHERE id = ? AND (stop_type IS NULL OR stop_type = '')");
for (const stay of stays) {
insert.run(stay.start_day_id, stay.place_id, stay.start_day_id, stay.id);
stamp.run(stay.place_id);
}
if (stays.length > 0) console.log(`[DB] Put ${stays.length} booked night(s) on their check-in day`);
},
];
if (currentVersion < migrations.length) {
server/src/nest/accommodations/accommodations.controller.ts
+6 -1
View File
@@ -2,6 +2,7 @@ import {
Body,
Controller,
Delete,
Query,
Get,
Headers,
HttpException,
@@ -117,11 +118,15 @@ export class AccommodationsController {
@Param('tripId') tripId: string,
@Param('id') id: string,
@Headers('x-socket-id') socketId?: string,
@Query('keepStop') keepStop?: string,
) {
if (!this.accommodations.get(id, tripId)) {
throw new HttpException({ error: 'Accommodation not found' }, 404);
}
const { linkedReservationIds, deletedBudgetItemIds, mirror } = this.accommodations.remove(id);
// Turning a night back into a pause in road trip mode: the booking goes, the
// stop stays and becomes the traveller's. Everywhere else a cancelled booking
// takes the stop it brought with it.
const { linkedReservationIds, deletedBudgetItemIds, mirror } = this.accommodations.remove(id, { keepStop: keepStop === 'true' });
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);
server/src/nest/accommodations/accommodations.service.ts
+19 -7
View File
@@ -123,8 +123,8 @@ export class AccommodationsService {
return this.updateAccommodation(id, existing, fields);
}
remove(id: string | number) {
return this.deleteAccommodation(id);
remove(id: string | number, opts: { keepStop?: boolean } = {}) {
return this.deleteAccommodation(id, opts);
}
/**
@@ -239,15 +239,18 @@ export class AccommodationsService {
/**
* 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.
* Only its own stop moves. A booking that owns none is one whose stop belongs to
* the traveller: booked in road trip mode, where the place was put on the day
* first, or booked for a place they had already planned there. Moving that is not
* ours to do, and putting a second one on the new day next to it is exactly the
* duplicate this whole change is meant to remove. Stays booked before any of this
* existed get their stop from the migration, not from the next edit.
*
* Runs inside the caller's transaction.
*/
private remirrorStay(accommodationId: number, placeId: number | null, dayId: number): 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();
const mirror = noMirror();
@@ -350,7 +353,7 @@ export class AccommodationsService {
* that no longer exists. `linkedReservationId` / `deletedBudgetItemId` stay on the
* result as the first of each, because the RPC, MCP and REST callers read them.
*/
deleteAccommodation(id: string | number): {
deleteAccommodation(id: string | number, opts: { keepStop?: boolean } = {}): {
linkedReservationId: number | null;
deletedBudgetItemId: number | null;
linkedReservationIds: number[];
@@ -372,8 +375,17 @@ export class AccommodationsService {
// 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.
//
// keepStop hands it to the traveller instead of taking it away. That is the
// road trip popup turning a night back into a pause: they asked to drop the
// booking, not the place, and the stop is mid-drive where re-adding it would
// land it at the end of the day.
const mirror = noMirror();
for (const stop of this.ownStops(Number(id))) {
if (opts.keepStop) {
this.db.run('UPDATE day_assignments SET accommodation_id = NULL WHERE id = ?', stop.id);
continue;
}
this.db.run('DELETE FROM day_assignments WHERE id = ?', stop.id);
mirror.removed.push({ id: stop.id, dayId: stop.day_id });
}
server/tests/unit/nest/accommodations.service.test.ts
+10 -7
View File
@@ -729,19 +729,22 @@ describe('the day stop a booking implies', () => {
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.
it('ACC-030 a stay whose stop belongs to the traveller does not get a second one', () => {
const { user } = createUser(testDb);
const trip = createTrip(testDb, user.id);
const day = createDay(testDb, trip.id);
const second = 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;
// The road trip books a night on a place it put on the day first, so the stop is
// the traveller's and the booking owns nothing.
createDayAssignment(testDb, day.id, place.id);
const stay = 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' });
const existing = svc.getAccommodation(stay.id, trip.id)!;
svc.updateAccommodation(stay.id, existing, { start_day_id: second.id, end_day_id: second.id });
expect(stopsOn(day.id)).toEqual([expect.objectContaining({ place_id: place.id, accommodation_id: old.id })]);
expect(stopsOn(second.id)).toEqual([]);
expect(stopsOn(day.id)).toEqual([expect.objectContaining({ place_id: place.id, accommodation_id: null })]);
});
it('ACC-031 cancelling a booking takes back its own stop and leaves the other one', () => {
shared/src/assignment/assignment.schema.ts
+5
View File
@@ -48,6 +48,11 @@ export const assignmentSchema = z.object({
// not a place (booking arrival / morning hotel). null = inherit the day default.
// Inert when the previous timeline element is a place.
incoming_leg_transport_mode: z.string().nullable().optional(),
// The lodging booking that put this stop on the day, when one did. The day
// planner already shows that booking as its own overnight block, so it hides
// this row and leaves it to road trip mode, which is the view that needs the
// hotel in the driving chain. Null for every stop a traveller placed.
accommodation_id: z.number().nullable().optional(),
participants: z.array(assignmentParticipantSchema).optional(),
created_at: z.string().optional(),
place: assignmentPlaceSchema,