diff --git a/server/src/db/repositories/ReservationDayPositions.repository.ts b/server/src/db/repositories/ReservationDayPositions.repository.ts index 14b7a18a2..99259e16f 100644 --- a/server/src/db/repositories/ReservationDayPositions.repository.ts +++ b/server/src/db/repositories/ReservationDayPositions.repository.ts @@ -32,6 +32,20 @@ export class ReservationDayPositionsRepository extends TrekRepository('all', false); } + /** + * `SELECT day_id, position FROM reservation_day_positions WHERE reservation_id = ?`: + * one booking's slots, for the single-booking read the create, update and + * broadcast payloads come from. Without them a booking moved on a day lost + * that slot in the list after every edit until the next reload (#2631). + */ + async listForReservation(reservation_id: number): Promise<{ day_id: number; position: number }[]> { + return this.kysely() + .selectFrom('reservation_day_positions') + .select(['day_id', 'position']) + .where('reservation_id', '=', reservation_id) + .execute(); + } + /** * RS31 (`ReservationsService.updatePositions`'s day branch) — `INSERT OR * REPLACE INTO reservation_day_positions (reservation_id, day_id, diff --git a/server/src/nest/reservations/reservations-read.module.ts b/server/src/nest/reservations/reservations-read.module.ts index 5374131dc..a5623fc89 100644 --- a/server/src/nest/reservations/reservations-read.module.ts +++ b/server/src/nest/reservations/reservations-read.module.ts @@ -1,3 +1,4 @@ +import { ReservationDayPositions } from '../../db/entities/ReservationDayPositions.entity'; import { ReservationEndpoints } from '../../db/entities/ReservationEndpoints.entity'; import { ReservationTravelers } from '../../db/entities/ReservationTravelers.entity'; import { Reservations } from '../../db/entities/Reservations.entity'; @@ -11,14 +12,17 @@ import { Module } from '@nestjs/common'; * in — which would re-close the cycle that airtrail.bridge used to dodge. * * `MikroOrmModule.forFeature([Reservations, ReservationEndpoints, - * ReservationTravelers])` registers `ReservationsRepository`/ - * `ReservationEndpointsRepository`/`ReservationTravelersRepository` for + * ReservationTravelers, ReservationDayPositions])` registers `ReservationsRepository`/ + * `ReservationEndpointsRepository`/`ReservationTravelersRepository`/ + * `ReservationDayPositionsRepository` for * `ReservationsReadService`'s `@InjectRepository` constructor params (Plan * 3d Task 2, R11) — the entity classes only, not `ReservationsModule` * itself, so this module stays the same leaf it always was. */ @Module({ - imports: [MikroOrmModule.forFeature([Reservations, ReservationEndpoints, ReservationTravelers])], + imports: [ + MikroOrmModule.forFeature([Reservations, ReservationEndpoints, ReservationTravelers, ReservationDayPositions]), + ], providers: [ReservationsReadService], exports: [ReservationsReadService], }) diff --git a/server/src/nest/reservations/reservations-read.service.ts b/server/src/nest/reservations/reservations-read.service.ts index 914241945..4bab9f412 100644 --- a/server/src/nest/reservations/reservations-read.service.ts +++ b/server/src/nest/reservations/reservations-read.service.ts @@ -1,6 +1,8 @@ +import { ReservationDayPositions } from '../../db/entities/ReservationDayPositions.entity'; import { ReservationEndpoints } from '../../db/entities/ReservationEndpoints.entity'; import { ReservationTravelers } from '../../db/entities/ReservationTravelers.entity'; import { Reservations } from '../../db/entities/Reservations.entity'; +import type { ReservationDayPositionsRepository } from '../../db/repositories/ReservationDayPositions.repository'; import type { ReservationEndpointsRepository } from '../../db/repositories/ReservationEndpoints.repository'; import type { ReservationTravelersRepository } from '../../db/repositories/ReservationTravelers.repository'; import type { ReservationsRepository } from '../../db/repositories/Reservations.repository'; @@ -53,6 +55,7 @@ export class ReservationsReadService { @InjectRepository(Reservations) private readonly reservationsRepo: ReservationsRepository, @InjectRepository(ReservationEndpoints) private readonly endpointsRepo: ReservationEndpointsRepository, @InjectRepository(ReservationTravelers) private readonly travelersRepo: ReservationTravelersRepository, + @InjectRepository(ReservationDayPositions) private readonly dayPositionsRepo: ReservationDayPositionsRepository, ) {} async getReservationWithJoins(id: string | number): Promise { @@ -62,6 +65,7 @@ export class ReservationsReadService { const result: ReservationRow = { ...row }; result.endpoints = await this.loadEndpoints(row.id); result.travelers = await this.loadTravelers(row.id); + result.day_positions = await this.loadDayPositions(row.id); // accommodation_id is a TEXT column; the integer FK reads back as a numeric // string (e.g. "14.0"). Normalize to an int so clients can parse it. result.accommodation_id = row.accommodation_id == null ? null : Math.trunc(Number(row.accommodation_id)); @@ -74,6 +78,17 @@ export class ReservationsReadService { return (await this.endpointsRepo.listForReservation(reservationId)) as ReservationEndpoint[]; } + /** + * The booking's slot per day, in the shape `ReservationsService.list` sends: + * `{ [day_id]: position }`, or null without any. The list reads it before the + * booking's time, so a payload without it put a moved booking back by the clock. + */ + async loadDayPositions(reservationId: number): Promise | null> { + const rows = await this.dayPositionsRepo.listForReservation(reservationId); + if (rows.length === 0) return null; + return Object.fromEntries(rows.map((dp) => [dp.day_id, dp.position])); + } + async loadTravelers(reservationId: number | string): Promise { // RR3 — no ORDER BY (unlike RS6), matching the legacy statement's own row order. const rows = await this.travelersRepo.listForReservation(Number(reservationId)); diff --git a/server/tests/e2e/reservations.e2e.test.ts b/server/tests/e2e/reservations.e2e.test.ts index a990d0f7a..bef8528d1 100644 --- a/server/tests/e2e/reservations.e2e.test.ts +++ b/server/tests/e2e/reservations.e2e.test.ts @@ -439,6 +439,23 @@ describe('Reservations + accommodations e2e (real auth guard + temp SQLite, real expect(await positionRow(ownReservationId, ownDayId)).toEqual({ position: 3 }); }); + // The list puts a booking where it was moved on a day before its time, and the + // client replaces the booking with what an edit answers (#2631). + it('keeps the day slot in what an edit of the booking answers', async () => { + const ownReservationId = await seedReservation(tripId, 'Mine', 'other'); + await request(server) + .put(`/api/trips/${tripId}/reservations/positions`) + .set('Cookie', sessionCookie(1)) + .send({ positions: [{ id: ownReservationId, day_plan_position: 2.5 }], day_id: ownDayId }) + .expect(200); + const res = await request(server) + .put(`/api/trips/${tripId}/reservations/${ownReservationId}`) + .set('Cookie', sessionCookie(1)) + .send({ notes: 'aisle' }); + expect(res.status).toBe(200); + expect(res.body.reservation.day_positions).toEqual({ [ownDayId]: 2.5 }); + }); + // An id that exists nowhere used to raise a foreign-key error, which the // exception filter turned into a 500 — telling the caller apart from the // 200 a real id returns, for any id on the instance. diff --git a/server/tests/helpers/plugin-host.ts b/server/tests/helpers/plugin-host.ts index 35e5da371..faafc6450 100644 --- a/server/tests/helpers/plugin-host.ts +++ b/server/tests/helpers/plugin-host.ts @@ -479,6 +479,7 @@ export async function createPluginRpcHostParts( await createTestReservationsRepo(db), await createTestReservationEndpointsRepo(db), await createTestReservationTravelersRepo(db), + await createTestReservationDayPositionsRepo(db), ), accommodations, await createTestUnitOfWork(db), diff --git a/server/tests/unit/db/repositories/ReservationDayPositions.repository.test.ts b/server/tests/unit/db/repositories/ReservationDayPositions.repository.test.ts index ffa09bf56..c2d0e83dc 100644 --- a/server/tests/unit/db/repositories/ReservationDayPositions.repository.test.ts +++ b/server/tests/unit/db/repositories/ReservationDayPositions.repository.test.ts @@ -75,4 +75,14 @@ describe('ReservationDayPositionsRepository — fully seeded world', () => { const trip = createTrip(testDb, user.id); expect(await repo.listForTrip(trip.id)).toEqual([]); }); + + it('listForReservation — the slots of one booking per day, none of another booking', async () => { + const { multiDay, day1, day2 } = await seed(); + const rows = await repo.listForReservation(multiDay.id); + expect(rows.map((r) => [r.day_id, r.position]).sort()).toEqual([ + [day1.id, 0], + [day2.id, 1], + ]); + expect(await repo.listForReservation(999999)).toEqual([]); + }); }); diff --git a/server/tests/unit/mcp/tools-reservations.test.ts b/server/tests/unit/mcp/tools-reservations.test.ts index a253903b9..316a3df0b 100644 --- a/server/tests/unit/mcp/tools-reservations.test.ts +++ b/server/tests/unit/mcp/tools-reservations.test.ts @@ -5,6 +5,7 @@ import { db as testDb } from '../../../src/db/database'; import { BudgetItems } from '../../../src/db/entities/BudgetItems.entity'; import { DayAccommodations } from '../../../src/db/entities/DayAccommodations.entity'; +import { ReservationDayPositions } from '../../../src/db/entities/ReservationDayPositions.entity'; import { ReservationTravelers } from '../../../src/db/entities/ReservationTravelers.entity'; import { Reservations } from '../../../src/db/entities/Reservations.entity'; import { Trips } from '../../../src/db/entities/Trips.entity'; @@ -293,6 +294,26 @@ describe('Tool: update_reservation', () => { expect(events.indexOf('budget:updated')).toBeLessThan(events.indexOf('reservation:updated')); }); + // #2631: the day planner puts a booking moved on a day by its day slot, and the + // edit's answer and broadcast replace the booking in every open list. + it('keeps the day slots in the answer and the broadcast', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id); + const reservation = createReservation(testDb, trip.id, { title: 'Bus', type: 'bus' }); + await insertRow(orm, ReservationDayPositions, { reservation: reservation.id, day: day.id, position: 2.5 }); + await withHarness(user.id, async (h) => { + const result = await h.client.callTool({ + name: 'update_reservation', + arguments: { tripId: trip.id, reservationId: reservation.id, notes: 'aisle' }, + }); + type Answer = { reservation: { day_positions: Record | null } }; + expect((parseToolResult(result) as Answer).reservation.day_positions).toEqual({ [day.id]: 2.5 }); + const sent = broadcastMock.mock.calls.find((c) => c[1] === 'reservation:updated')?.[2] as Answer; + expect(sent.reservation.day_positions).toEqual({ [day.id]: 2.5 }); + }); + }); + it('returns error for reservation not found', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); diff --git a/server/tests/unit/nest/calendar.service.test.ts b/server/tests/unit/nest/calendar.service.test.ts index 1541cb149..1256ae262 100644 --- a/server/tests/unit/nest/calendar.service.test.ts +++ b/server/tests/unit/nest/calendar.service.test.ts @@ -58,7 +58,7 @@ let orm: TestOrm; beforeAll(async () => { budgetSvc = new BudgetService(new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), new ExchangeRatesService(), realtime, await createTestUnitOfWork(testDb), ...(await budgetRepoArgs(testDb))); svc = new CalendarService( - new ReservationsService(new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), budgetSvc, realtime, notificationsStub(), new ReservationsReadService(await createTestReservationsRepo(testDb), await createTestReservationEndpointsRepo(testDb), await createTestReservationTravelersRepo(testDb)), await accommodationsOver(testDb, realtime), await createTestUnitOfWork(testDb), await createTestReservationsRepo(testDb), await createTestReservationEndpointsRepo(testDb), await createTestReservationTravelersRepo(testDb), await createTestReservationDayPositionsRepo(testDb), await createTestDayAccommodationsRepo(testDb), await createTestDaysRepo(testDb), await createTestPlacesRepo(testDb), await createTestDayAssignmentsRepo(testDb), await createTestTripMembersRepo(testDb), await createTestUsersRepo(testDb), await createTestTripsRepo(testDb), await createTestBudgetItemsRepo(testDb)), + new ReservationsService(new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), budgetSvc, realtime, notificationsStub(), new ReservationsReadService(await createTestReservationsRepo(testDb), await createTestReservationEndpointsRepo(testDb), await createTestReservationTravelersRepo(testDb), await createTestReservationDayPositionsRepo(testDb)), await accommodationsOver(testDb, realtime), await createTestUnitOfWork(testDb), await createTestReservationsRepo(testDb), await createTestReservationEndpointsRepo(testDb), await createTestReservationTravelersRepo(testDb), await createTestReservationDayPositionsRepo(testDb), await createTestDayAccommodationsRepo(testDb), await createTestDaysRepo(testDb), await createTestPlacesRepo(testDb), await createTestDayAssignmentsRepo(testDb), await createTestTripMembersRepo(testDb), await createTestUsersRepo(testDb), await createTestTripsRepo(testDb), await createTestBudgetItemsRepo(testDb)), await createTestTripsRepo(testDb), await createTestDaysRepo(testDb), await createTestDayNotesRepo(testDb), await createTestReservationsRepo(testDb), ); }); diff --git a/server/tests/unit/nest/integrations/airtrailImport.test.ts b/server/tests/unit/nest/integrations/airtrailImport.test.ts index fcca3ac28..ed845d654 100644 --- a/server/tests/unit/nest/integrations/airtrailImport.test.ts +++ b/server/tests/unit/nest/integrations/airtrailImport.test.ts @@ -73,6 +73,7 @@ async function makeImportService(): Promise { await createTestReservationsRepo(db), await createTestReservationEndpointsRepo(db), await createTestReservationTravelersRepo(db), + await createTestReservationDayPositionsRepo(db), ), await accommodationsOver(db), await createTestUnitOfWork(db), diff --git a/server/tests/unit/nest/reservations.service.test.ts b/server/tests/unit/nest/reservations.service.test.ts index 05e1c02f9..ab09fc2a8 100644 --- a/server/tests/unit/nest/reservations.service.test.ts +++ b/server/tests/unit/nest/reservations.service.test.ts @@ -80,7 +80,7 @@ const broadcast = realtime.broadcastMock; let svc: ReservationsService; let orm: TestOrm; beforeAll(async () => { - svc = new ReservationsService(permissionsStub, budget as unknown as BudgetService, realtime, notificationsStub(notif.send), new ReservationsReadService(await createTestReservationsRepo(testDb), await createTestReservationEndpointsRepo(testDb), await createTestReservationTravelersRepo(testDb)), await makeAccommodationsService(testDb, realtime), await createTestUnitOfWork(testDb), await createTestReservationsRepo(testDb), await createTestReservationEndpointsRepo(testDb), await createTestReservationTravelersRepo(testDb), await createTestReservationDayPositionsRepo(testDb), await createTestDayAccommodationsRepo(testDb), await createTestDaysRepo(testDb), await createTestPlacesRepo(testDb), await createTestDayAssignmentsRepo(testDb), await createTestTripMembersRepo(testDb), await createTestUsersRepo(testDb), await createTestTripsRepo(testDb), await createTestBudgetItemsRepo(testDb)); + svc = new ReservationsService(permissionsStub, budget as unknown as BudgetService, realtime, notificationsStub(notif.send), new ReservationsReadService(await createTestReservationsRepo(testDb), await createTestReservationEndpointsRepo(testDb), await createTestReservationTravelersRepo(testDb), await createTestReservationDayPositionsRepo(testDb)), await makeAccommodationsService(testDb, realtime), await createTestUnitOfWork(testDb), await createTestReservationsRepo(testDb), await createTestReservationEndpointsRepo(testDb), await createTestReservationTravelersRepo(testDb), await createTestReservationDayPositionsRepo(testDb), await createTestDayAccommodationsRepo(testDb), await createTestDaysRepo(testDb), await createTestPlacesRepo(testDb), await createTestDayAssignmentsRepo(testDb), await createTestTripMembersRepo(testDb), await createTestUsersRepo(testDb), await createTestTripsRepo(testDb), await createTestBudgetItemsRepo(testDb)); }); beforeEach(async () => { @@ -387,6 +387,23 @@ describe('ReservationsService (DI-native, real SQL)', () => { const single = (await svc.getReservationWithJoins(res.id))!; expect(single.accommodation_id).toBe(acc.id); expect(single.travelers).toHaveLength(1); + expect(single.day_positions).toEqual({ [days[0].id]: 2 }); + }); + + // The create and update responses and their broadcasts come from the single read. + // Without the day slots in it, a booking moved on a day went back to its time in + // the list after every edit until the next reload (#2631). + it('RESV-SVC-015b: the update result keeps the day slot, null for a booking without one', async () => { + const { trip } = ownerTrip({ start_date: '2030-05-01', end_date: '2030-05-02' }); + const days = await findRows(orm, Days, { trip: trip.id }, { day_number: 'asc' }) as { id: number }[]; + const moved = createReservation(testDb, trip.id, { title: 'Train', type: 'train' }); + const untouched = createReservation(testDb, trip.id, { title: 'Bus', type: 'bus' }); + await svc.updatePositions(String(trip.id), [{ id: moved.id, day_plan_position: 3.5 }], days[1].id); + + const current = await svc.getReservation(String(moved.id), String(trip.id)); + const { reservation } = await svc.update(String(moved.id), String(trip.id), { notes: 'window seat' }, current as Parameters[3]); + expect(reservation.day_positions).toEqual({ [days[1].id]: 3.5 }); + expect((await svc.getReservationWithJoins(untouched.id))?.day_positions).toBeNull(); }); it('RESV-SVC-016: getReservationWithJoins returns undefined for a missing id', async () => { diff --git a/server/tests/unit/nest/trip-read-model.service.test.ts b/server/tests/unit/nest/trip-read-model.service.test.ts index 7a0c97cbe..c629cd2e4 100644 --- a/server/tests/unit/nest/trip-read-model.service.test.ts +++ b/server/tests/unit/nest/trip-read-model.service.test.ts @@ -296,6 +296,7 @@ const buildReadModel = async (tripsRepo: TripsRepository, roster: TripMembersSer await createTestReservationsRepo(testDb), await createTestReservationEndpointsRepo(testDb), await createTestReservationTravelersRepo(testDb), + await createTestReservationDayPositionsRepo(testDb), ), await accommodationsOver(testDb, realtime), await createTestUnitOfWork(testDb), diff --git a/server/tests/unit/nest/trips.service.test.ts b/server/tests/unit/nest/trips.service.test.ts index 1ac9c9f51..ee5d1de3f 100644 --- a/server/tests/unit/nest/trips.service.test.ts +++ b/server/tests/unit/nest/trips.service.test.ts @@ -278,7 +278,7 @@ beforeAll(async () => { buildPlaceImportService(), ); svc = new TripsService( - new ReservationsService(new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), budgetSvc, realtime, notificationsStub(), new ReservationsReadService(await createTestReservationsRepo(testDb), await createTestReservationEndpointsRepo(testDb), await createTestReservationTravelersRepo(testDb)), accommodationsSvc, await createTestUnitOfWork(testDb), await createTestReservationsRepo(testDb), await createTestReservationEndpointsRepo(testDb), await createTestReservationTravelersRepo(testDb), await createTestReservationDayPositionsRepo(testDb), await createTestDayAccommodationsRepo(testDb), await createTestDaysRepo(testDb), await createTestPlacesRepo(testDb), await createTestDayAssignmentsRepo(testDb), await createTestTripMembersRepo(testDb), await createTestUsersRepo(testDb), await createTestTripsRepo(testDb), await createTestBudgetItemsRepo(testDb)), + new ReservationsService(new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), budgetSvc, realtime, notificationsStub(), new ReservationsReadService(await createTestReservationsRepo(testDb), await createTestReservationEndpointsRepo(testDb), await createTestReservationTravelersRepo(testDb), await createTestReservationDayPositionsRepo(testDb)), accommodationsSvc, await createTestUnitOfWork(testDb), await createTestReservationsRepo(testDb), await createTestReservationEndpointsRepo(testDb), await createTestReservationTravelersRepo(testDb), await createTestReservationDayPositionsRepo(testDb), await createTestDayAccommodationsRepo(testDb), await createTestDaysRepo(testDb), await createTestPlacesRepo(testDb), await createTestDayAssignmentsRepo(testDb), await createTestTripMembersRepo(testDb), await createTestUsersRepo(testDb), await createTestTripsRepo(testDb), await createTestBudgetItemsRepo(testDb)), daysSvc, new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), budgetSvc, @@ -307,7 +307,7 @@ beforeAll(async () => { await createTestPackingCategoryAssigneesRepo(testDb), await createTestPackingTemplatesRepo(testDb), await createTestPackingTemplateCategoriesRepo(testDb), await createTestPackingTemplateItemsRepo(testDb), await createTestTripsRepo(testDb), await createTestTripMembersRepo(testDb), ), - new ReservationsService(new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), budgetSvc, realtime, notificationsStub(), new ReservationsReadService(await createTestReservationsRepo(testDb), await createTestReservationEndpointsRepo(testDb), await createTestReservationTravelersRepo(testDb)), accommodationsSvc, await createTestUnitOfWork(testDb), await createTestReservationsRepo(testDb), await createTestReservationEndpointsRepo(testDb), await createTestReservationTravelersRepo(testDb), await createTestReservationDayPositionsRepo(testDb), await createTestDayAccommodationsRepo(testDb), await createTestDaysRepo(testDb), await createTestPlacesRepo(testDb), await createTestDayAssignmentsRepo(testDb), await createTestTripMembersRepo(testDb), await createTestUsersRepo(testDb), await createTestTripsRepo(testDb), await createTestBudgetItemsRepo(testDb)), + new ReservationsService(new PermissionsService(await createTestAppSettingsRepo(testDb), await createTestUnitOfWork(testDb)), budgetSvc, realtime, notificationsStub(), new ReservationsReadService(await createTestReservationsRepo(testDb), await createTestReservationEndpointsRepo(testDb), await createTestReservationTravelersRepo(testDb), await createTestReservationDayPositionsRepo(testDb)), accommodationsSvc, await createTestUnitOfWork(testDb), await createTestReservationsRepo(testDb), await createTestReservationEndpointsRepo(testDb), await createTestReservationTravelersRepo(testDb), await createTestReservationDayPositionsRepo(testDb), await createTestDayAccommodationsRepo(testDb), await createTestDaysRepo(testDb), await createTestPlacesRepo(testDb), await createTestDayAssignmentsRepo(testDb), await createTestTripMembersRepo(testDb), await createTestUsersRepo(testDb), await createTestTripsRepo(testDb), await createTestBudgetItemsRepo(testDb)), new CollabService( // Plan 4 Task 2 — CollabService's own DatabaseService param is gone: // canAccessTrip now reads through the TripsRepository at the end.