fix(reservations): a night booked on the form belongs on the drive too

The booking form wrote the stay row itself, so it was the one door that left the
day stop out. A hotel entered there stayed invisible to road trip mode, which is
the same complaint as before with a different way in: enter the place twice or it
is not on the route.

It writes the stop now, through AccommodationsService rather than a second copy of
its SQL. The permission argument for not doing so does not hold: the night already
shows in the day header the moment the booking is saved, and the stop is that same
night in the other view, not a second edit to the plan.

Two holes go with it. Moving the booking left the stop on a day nobody sleeps there
any more, and deleting it left the stop behind entirely — in both cases still
carrying the booking's id, which is what the day list hides on, so nobody could
reach it to remove it. The stop now follows the stay and goes with it, and only
once the trip_id guard has confirmed the stay was this trip's to delete.
This commit is contained in:
Maurice
2026-09-13 12:38:21 +02:00
parent eff4605ef3
commit e3471e2dc8
12 changed files with 295 additions and 52 deletions
+11 -1
View File
@@ -17,7 +17,17 @@ import { TrekPhotosRepository } from '../../src/nest/photos/trek-photos.reposito
* why this is a helper and not five copies across the suites.
*/
export function makeAccommodationsService(conn: Database): AccommodationsService {
const dbs = new DatabaseService(conn);
return accommodationsOver(new DatabaseService(conn));
}
/**
* The same, for a suite that already holds the DatabaseService.
*
* ReservationsService takes one now, because a hotel booking owes the day plan
* the same stop a night entered under Days does, and every suite that builds
* that service by hand needs one to hand it.
*/
export function accommodationsOver(dbs: DatabaseService): AccommodationsService {
const permissions = new PermissionsService(dbs);
const realtime = new RealtimeService();
const assignments = new AssignmentsService(
+2 -1
View File
@@ -167,11 +167,12 @@ export function createMcpTestRegistry(): McpRegistry {
journeyDomain,
generalStorage,
);
const reservationsService = new ReservationsService(dbService, permissionsService, budgetService, realtimeService, notificationsStub(), new ReservationsReadRepository(dbService));
// 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);
// Built after it: a hotel booking writes the stay's day stop through this one.
const reservationsService = new ReservationsService(dbService, permissionsService, budgetService, realtimeService, notificationsStub(), new ReservationsReadRepository(dbService), accommodationsService);
const membersService = new TripMembersService(dbService, budgetService, new UserCleanupService(dbService, budgetService), permissionsService, realtimeService, notificationsStub());
const tripsService = new TripsService(
dbService,
+2 -1
View File
@@ -94,7 +94,6 @@ export function createPluginRpcHostFactory(dbs: DatabaseService): PluginRpcHostF
const todos = new TodoService(dbs, permissions, realtime);
const packing = new PackingService(dbs, permissions, realtime, notificationsStub());
const files = new FilesService(dbs, permissions, realtime, new EphemeralTokenService(), generalStorage);
const reservations = new ReservationsService(dbs, permissions, budget, realtime, notificationsStub(), new ReservationsReadRepository(dbs));
const collab = new CollabService(dbs, permissions, realtime, notificationsStub(), generalStorage, new RateLimitService());
const vacay = new VacayService(dbs, realtime, notificationsStub());
const days = new DaysService(dbs, permissions, realtime, queryHelpers);
@@ -111,6 +110,8 @@ export function createPluginRpcHostFactory(dbs: DatabaseService): PluginRpcHostF
const llmConfig = new LlmConfigResolver(new SettingsService(dbs), dbs, addons);
const oauth = new PluginOAuthService(dbs);
const accommodations = new AccommodationsService(dbs, permissions, realtime, assignments);
// After accommodations: a hotel booking writes the stay's day stop through it.
const reservations = new ReservationsService(dbs, permissions, budget, realtime, notificationsStub(), new ReservationsReadRepository(dbs), accommodations);
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);
@@ -56,6 +56,7 @@ import { CalendarService, foldICS } from '../../../src/nest/calendar/calendar.se
import { CalendarModule } from '../../../src/nest/calendar/calendar.module';
import { expectRegisteredProvider } from '../../helpers/module-providers';
import { notificationsStub } from '../../helpers/notifications';
import { accommodationsOver } from '../../helpers/accommodations-service';
const dbs = () => new DatabaseService(testDb);
const budgetSvc = new BudgetService(dbs(), new PermissionsService(dbs()), new ExchangeRatesService(), new RealtimeService());
@@ -63,7 +64,7 @@ const budgetSvc = new BudgetService(dbs(), new PermissionsService(dbs()), new Ex
// Named `svc` so the moved cases below read exactly as they did on TripsService.
const svc = new CalendarService(
dbs(),
new ReservationsService(dbs(), new PermissionsService(dbs()), budgetSvc, new RealtimeService(), notificationsStub(), new ReservationsReadRepository(dbs())),
new ReservationsService(dbs(), new PermissionsService(dbs()), budgetSvc, new RealtimeService(), notificationsStub(), new ReservationsReadRepository(dbs()), accommodationsOver(dbs())),
);
beforeAll(() => {
@@ -74,8 +74,9 @@ const bridge = {
};
import { RealtimeService } from '../../../src/nest/realtime/realtime.service';
import { notificationsStub } from '../../helpers/notifications';
import { makeAccommodationsService } from '../../helpers/accommodations-service';
const svc = new ReservationsService(new DatabaseService(testDb), permissionsStub, budget as unknown as BudgetService, new RealtimeService(), notificationsStub(notif.send), new ReservationsReadRepository(new DatabaseService(testDb)));
const svc = new ReservationsService(new DatabaseService(testDb), permissionsStub, budget as unknown as BudgetService, new RealtimeService(), notificationsStub(notif.send), new ReservationsReadRepository(new DatabaseService(testDb)), makeAccommodationsService(testDb));
beforeAll(() => { createTables(testDb); runMigrations(testDb); });
beforeEach(() => {
@@ -1054,3 +1055,131 @@ describe('ReservationsService — referenced ids stay inside the trip', () => {
expect(testDb.prepare('SELECT accommodation_id FROM reservations WHERE id = ?').get(res.id)).toEqual({ accommodation_id: null });
});
});
describe('the day stop a hotel booking implies', () => {
// Booking a night on this form is booking a night: it shows in the day header
// exactly like one entered under Days, and road trip mode draws the same booking
// as a service stop instead. Before this the booking form was the one door that
// wrote the stay without the stop, so a hotel entered here was invisible to the
// drive and had to be added a second time as an ordinary place.
const stopsOn = (dayId: number) =>
testDb.prepare('SELECT id, place_id, accommodation_id FROM day_assignments WHERE day_id = ? ORDER BY order_index').all(dayId) as
{ id: number; place_id: number; accommodation_id: number | null }[];
const tripWithDays = () => {
const { trip } = ownerTrip({ start_date: '2030-05-01', end_date: '2030-05-03' });
const days = testDb.prepare('SELECT id FROM days WHERE trip_id = ? ORDER BY day_number').all(trip.id) as { id: number }[];
return { trip, days };
};
it('RESV-STAY-001: a hotel booked on the form lands on its check-in day', () => {
const { trip, days } = tripWithDays();
const place = createPlace(testDb, trip.id, { name: 'Hotel Adlon' });
const { reservation } = svc.create(String(trip.id), {
title: 'Hotel', type: 'hotel',
create_accommodation: { place_id: place.id, start_day_id: days[0].id, end_day_id: days[2].id },
});
expect(stopsOn(days[0].id)).toEqual([expect.objectContaining({ place_id: place.id, accommodation_id: Number(reservation.accommodation_id) })]);
// Typed as lodging, so the road trip draws it as the service stop it is.
expect(testDb.prepare('SELECT stop_type FROM places WHERE id = ?').get(place.id)).toMatchObject({ stop_type: 'hotel' });
// Only the check-in day, the same as every other door.
expect(stopsOn(days[2].id)).toEqual([]);
});
it('RESV-STAY-002: a booking that is not a hotel puts nothing on the plan', () => {
const { trip, days } = tripWithDays();
svc.create(String(trip.id), { title: 'Museum tour', type: 'tour', day_id: days[0].id });
expect(stopsOn(days[0].id)).toEqual([]);
});
it('RESV-STAY-003: moving the booking to another day takes its stop along', () => {
const { trip, days } = tripWithDays();
const place = createPlace(testDb, trip.id, { name: 'Hotel Adlon' });
const { reservation } = svc.create(String(trip.id), {
title: 'Hotel', type: 'hotel',
create_accommodation: { place_id: place.id, start_day_id: days[0].id, end_day_id: days[0].id },
});
const current = svc.getReservation(String(reservation.id), String(trip.id))!;
svc.update(String(reservation.id), String(trip.id), {
type: 'hotel',
create_accommodation: { place_id: place.id, start_day_id: days[1].id, end_day_id: days[2].id },
} as never, current);
// Left behind it would sit on a day nobody sleeps there, hidden from the day
// list because it still carries this booking's id.
expect(stopsOn(days[0].id)).toEqual([]);
expect(stopsOn(days[1].id)).toEqual([expect.objectContaining({ place_id: place.id, accommodation_id: Number(reservation.accommodation_id) })]);
});
it('RESV-STAY-004: an edit that first creates the stay writes the stop too', () => {
// A booking saved without a date range, then given one. resolvedAccId is null on
// the way in, so this takes the insert branch rather than the update branch.
const { trip, days } = tripWithDays();
const place = createPlace(testDb, trip.id, { name: 'Hotel Adlon' });
const { reservation } = svc.create(String(trip.id), { title: 'Hotel', type: 'hotel' });
const current = svc.getReservation(String(reservation.id), String(trip.id))!;
svc.update(String(reservation.id), String(trip.id), {
type: 'hotel',
create_accommodation: { place_id: place.id, start_day_id: days[1].id, end_day_id: days[1].id },
} as never, current);
const stay = testDb.prepare('SELECT id FROM day_accommodations WHERE trip_id = ?').get(trip.id) as { id: number };
expect(stopsOn(days[1].id)).toEqual([expect.objectContaining({ place_id: place.id, accommodation_id: stay.id })]);
});
it('RESV-STAY-005: deleting the booking takes the stop with the stay', () => {
const { trip, days } = tripWithDays();
const place = createPlace(testDb, trip.id, { name: 'Hotel Adlon' });
const { reservation } = svc.create(String(trip.id), {
title: 'Hotel', type: 'hotel',
create_accommodation: { place_id: place.id, start_day_id: days[0].id, end_day_id: days[0].id },
});
const { accommodationDeleted } = svc.remove(String(reservation.id), String(trip.id));
expect(accommodationDeleted).toBe(true);
// A stop still carrying a dead booking's id is one the day list hides and
// nobody can reach to remove.
expect(stopsOn(days[0].id)).toEqual([]);
});
it('RESV-STAY-006: a place the traveller already planned that day keeps its own row', () => {
const { trip, days } = tripWithDays();
const place = createPlace(testDb, trip.id, { name: 'Hotel Adlon' });
const own = testDb.prepare('INSERT INTO day_assignments (day_id, place_id, order_index) VALUES (?, ?, 0)').run(days[0].id, place.id);
const { reservation } = svc.create(String(trip.id), {
title: 'Hotel', type: 'hotel',
create_accommodation: { place_id: place.id, start_day_id: days[0].id, end_day_id: days[0].id },
});
// No second row, and the booking claims neither: cancelling it must not delete
// a stop the traveller placed.
expect(stopsOn(days[0].id)).toEqual([{ id: Number(own.lastInsertRowid), place_id: place.id, accommodation_id: null }]);
svc.remove(String(reservation.id), String(trip.id));
expect(stopsOn(days[0].id)).toHaveLength(1);
});
it('RESV-STAY-007: a foreign accommodation_id reaches no stop on the other trip', () => {
// The trip_id guard on the stay delete is what denies that reach; the stops go
// by accommodation id alone and would otherwise follow it straight over.
const { user: attacker } = createUser(testDb);
const { user: victim } = createUser(testDb, { email: 'victim@example.test' });
const mine = createTrip(testDb, attacker.id, { start_date: '2030-05-01', end_date: '2030-05-03' });
const theirs = createTrip(testDb, victim.id, { start_date: '2030-05-01', end_date: '2030-05-03' });
const foreignPlace = createPlace(testDb, theirs.id, { name: 'Their hotel' });
const foreignDay = testDb.prepare('SELECT id FROM days WHERE trip_id = ? ORDER BY day_number').get(theirs.id) as { id: number };
const foreignAcc = createDayAccommodation(testDb, theirs.id, foreignPlace.id, foreignDay.id, foreignDay.id);
const theirStop = testDb.prepare('INSERT INTO day_assignments (day_id, place_id, order_index, accommodation_id) VALUES (?, ?, 0, ?)')
.run(foreignDay.id, foreignPlace.id, foreignAcc.id);
const res = createReservation(testDb, mine.id, { title: 'Hotel', type: 'hotel' });
testDb.prepare('UPDATE reservations SET accommodation_id = ? WHERE id = ?').run(foreignAcc.id, res.id);
svc.remove(String(res.id), String(mine.id));
expect(testDb.prepare('SELECT id FROM day_assignments WHERE id = ?').get(Number(theirStop.lastInsertRowid))).toBeTruthy();
});
});
@@ -70,6 +70,7 @@ 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';
import { accommodationsOver } from '../../helpers/accommodations-service';
import { EphemeralTokenService } from '../../../src/nest/auth/ephemeral-token.service';
import { UnsplashService } from '../../../src/nest/unsplash/unsplash.service';
import { PlacePhotoCacheService } from '../../../src/nest/place-photos/place-photo-cache.service';
@@ -101,7 +102,7 @@ const buildReadModel = (database: DatabaseService, roster: TripMembersService =
new TripReadModelService(
database, roster, daysSvc, accommodationsSvc, budgetSvc,
new PackingService(dbs(), new PermissionsService(dbs()), new RealtimeService(), notificationsStub()),
new ReservationsService(dbs(), new PermissionsService(dbs()), budgetSvc, new RealtimeService(), notificationsStub(), new ReservationsReadRepository(dbs())),
new ReservationsService(dbs(), new PermissionsService(dbs()), budgetSvc, new RealtimeService(), notificationsStub(), new ReservationsReadRepository(dbs()), accommodationsOver(dbs())),
new CollabService(dbs(), new PermissionsService(dbs()), new RealtimeService(), notificationsStub(), makeStorageFixture('').storage, new RateLimitService()),
placesSvc,
new TodoService(dbs(), new PermissionsService(dbs()), new RealtimeService()),
+3 -3
View File
@@ -118,7 +118,7 @@ const createAccommodation = accommodationsSvc.createAccommodation.bind(accommoda
const svc = new TripsService(
dbs(),
new ReservationsService(dbs(), new PermissionsService(dbs()), budgetSvc, new RealtimeService(), notificationsStub(), new ReservationsReadRepository(dbs())),
new ReservationsService(dbs(), new PermissionsService(dbs()), budgetSvc, new RealtimeService(), notificationsStub(), new ReservationsReadRepository(dbs()), accommodationsSvc),
daysSvc,
new PermissionsService(dbs()),
budgetSvc,
@@ -131,7 +131,7 @@ const membersSvc = new TripMembersService(dbs(), budgetSvc, new UserCleanupServi
const readModelSvc = new TripReadModelService(
dbs(), membersSvc, daysSvc, accommodationsSvc, budgetSvc,
new PackingService(dbs(), new PermissionsService(dbs()), new RealtimeService(), notificationsStub()),
new ReservationsService(dbs(), new PermissionsService(dbs()), budgetSvc, new RealtimeService(), notificationsStub(), new ReservationsReadRepository(dbs())),
new ReservationsService(dbs(), new PermissionsService(dbs()), budgetSvc, new RealtimeService(), notificationsStub(), new ReservationsReadRepository(dbs()), accommodationsSvc),
new CollabService(dbs(), new PermissionsService(dbs()), new RealtimeService(), notificationsStub(), coversFx.storage, new RateLimitService()),
placesSvc,
new TodoService(dbs(), new PermissionsService(dbs()), new RealtimeService()),
@@ -1098,7 +1098,7 @@ describe('quirk fixes', () => {
const fdbs = failingConnection(match);
return new TripsService(
fdbs,
new ReservationsService(dbs(), new PermissionsService(dbs()), budgetSvc, new RealtimeService(), notificationsStub(), new ReservationsReadRepository(dbs())),
new ReservationsService(dbs(), new PermissionsService(dbs()), budgetSvc, new RealtimeService(), notificationsStub(), new ReservationsReadRepository(dbs()), accommodationsSvc),
daysSvc,
new PermissionsService(dbs()),
budgetSvc,
@@ -22,6 +22,7 @@ import { ReservationsReadRepository } from '../../../src/nest/reservations/reser
import type { AirtrailClient } from '../../../src/nest/integrations/airtrail.client';
import type { AirtrailService } from '../../../src/nest/integrations/airtrail.service';
import { notificationsStub } from '../../helpers/notifications';
import { accommodationsOver } from '../../helpers/accommodations-service';
// The client and the per-user credentials are the only stubs; the reservation
// writes go through the real service against the real test DB, as before. They
@@ -43,6 +44,7 @@ function makeImportService(): AirtrailImportService {
realtime,
notificationsStub(),
new ReservationsReadRepository(dbs()),
accommodationsOver(dbs()),
),
{ listFlights } as unknown as AirtrailClient,
{