From d0d3f455a80a9d03447b777c68fefe46e5ec1f3e Mon Sep 17 00:00:00 2001 From: Maurice Date: Sun, 13 Sep 2026 12:59:19 +0200 Subject: [PATCH] fix(planner): let the day list see which stop a booking wrote The row shapers name every column they pass on, and accommodation_id was in none of them. So the field the day list hides on never reached the client, and the booked night was listed as an ordinary place next to the overnight block that already named it. The stop was correct in the table the whole time; the three read paths just dropped it on the way out. The stamp on the freshly written stop had the same hole from the other side: the INSERT went in without the booking id and an UPDATE added it afterwards, so the row handed back in the answer, the one the client that booked the night puts straight into its store, carried none. It goes in with the INSERT now. The shared link gains the same filter. It has no road trip view to need the stop and already shows the booking as its own chip on the day, so the row was only ever the same hotel twice. --- client/src/pages/SharedTripPage.test.tsx | 33 +++++++++++++++++++ client/src/pages/SharedTripPage.tsx | 6 +++- .../accommodations/accommodations.service.ts | 12 ++++--- .../nest/assignments/assignments.service.ts | 13 ++++++-- server/src/nest/common/rowShape.ts | 4 +++ server/src/nest/days/days.service.ts | 3 ++ server/src/nest/share/share.service.ts | 3 ++ server/src/types.ts | 2 ++ .../unit/nest/accommodations.service.test.ts | 5 +++ 9 files changed, 72 insertions(+), 9 deletions(-) diff --git a/client/src/pages/SharedTripPage.test.tsx b/client/src/pages/SharedTripPage.test.tsx index 68efc409a..40c3f770a 100644 --- a/client/src/pages/SharedTripPage.test.tsx +++ b/client/src/pages/SharedTripPage.test.tsx @@ -1353,6 +1353,39 @@ describe('SharedTripPage', () => { }); }); + describe('FE-PAGE-SHARED-037b: a booked night is not listed twice on a shared day', () => { + // Booking a night puts its hotel on the check-in day as a stop, because road trip + // mode drives to it. A shared link has no road trip view and already shows the + // booking as its own chip on the day, so that stop would be the same hotel again. + const days = [{ id: 41, trip_id: 1, day_number: 1, date: '2026-07-01', title: 'Day One' }]; + // The address is the tell: the list row prints it, the map marker's tooltip does not, + // so it says whether the row rendered rather than whether the place exists at all. + const place = (id: number, name: string, address: string) => ({ + id, name, description: null, address, lat: 48.86, lng: 2.34, category_id: null, + price: null, place_time: null, end_time: null, duration_minutes: null, notes: null, + website: null, phone: null, image_url: null, transport_mode: null, category: null, tags: [], + }); + + it('leaves out the stop the booking wrote and keeps the one the traveller placed', async () => { + await open('stay-token', payload({ + days, + assignments: { + 41: [ + { id: 401, day_id: 41, order_index: 0, notes: null, accommodation_id: null, place: place(601, 'Musee Rodin', '77 Rue de Varenne') }, + { id: 402, day_id: 41, order_index: 1, notes: null, accommodation_id: 7, place: place(602, 'Hotel Adlon', 'Unter den Linden 77') }, + ], + }, + accommodations: [{ id: 7, place_id: 602, start_day_id: 41, end_day_id: 41, place_name: 'Hotel Adlon' }], + })); + + fireEvent.click(screen.getByText('Day One')); + // The stop the traveller placed is listed, address and all. + await waitFor(() => expect(screen.getByText('77 Rue de Varenne')).toBeInTheDocument()); + // The booked night is not: it is already on the day as its own chip. + expect(screen.queryByText('Unter den Linden 77')).toBeNull(); + }); + }); + // ── #2320: the richer read-only detail ───────────────────────────────── describe('FE-PAGE-SHARED-038: a stop shows its address, description, both notes, duration and contact (#2320)', () => { diff --git a/client/src/pages/SharedTripPage.tsx b/client/src/pages/SharedTripPage.tsx index c9dadaf61..9e04a494f 100644 --- a/client/src/pages/SharedTripPage.tsx +++ b/client/src/pages/SharedTripPage.tsx @@ -465,7 +465,11 @@ export default function SharedTripPage() { {/* Day Plan */}
{sortedDays.map((day: any, di: number) => { - const da = assignments[String(day.id)] || []; + // Without the booked nights: the day already shows each of them as its + // own chip, and the stop a booking writes for the drive would be that + // same hotel a second time. There is no road trip view on a shared link, + // so the stop has nothing else to do here. + const da = (assignments[String(day.id)] || []).filter((a: any) => a.accommodation_id == null); // A share can still carry an assignment for a deleted place. The timeline // skips those rows, so the header must not count them either. const dayPlaceCount = da.filter((a: any) => a.place).length; diff --git a/server/src/nest/accommodations/accommodations.service.ts b/server/src/nest/accommodations/accommodations.service.ts index 409d74708..4e4f5d37e 100644 --- a/server/src/nest/accommodations/accommodations.service.ts +++ b/server/src/nest/accommodations/accommodations.service.ts @@ -255,11 +255,13 @@ export class AccommodationsService { // 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. - const created = this.assignments.createAssignment(dayId, placeId); - if (created) { - this.db.run('UPDATE day_assignments SET accommodation_id = ? WHERE id = ?', accommodationId, created.id); - mirror.created = created; - } + // + // 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 + // would leave that copy without it. The day list has nothing else to tell the + // stop from a place the traveller added, so it would show the hotel a second + // time until the next reload. + mirror.created = this.assignments.createAssignment(dayId, placeId, null, { accommodationId }); return mirror; } diff --git a/server/src/nest/assignments/assignments.service.ts b/server/src/nest/assignments/assignments.service.ts index 0021283db..8170ff50a 100644 --- a/server/src/nest/assignments/assignments.service.ts +++ b/server/src/nest/assignments/assignments.service.ts @@ -124,14 +124,21 @@ export class AssignmentsService { return !!this.dbs.get('SELECT id FROM places WHERE id = ? AND trip_id = ?', placeId, tripId); } - createAssignment(dayId: string | number, placeId: unknown, notes?: string | null) { + /** + * @param opts.accommodationId The lodging booking this stop belongs to, when a + * booking is what put it there. Written by the INSERT rather than stamped on + * afterwards: the row this returns is what the answer hands the client, and a + * stop that reaches it without its booking id is one the day list cannot tell + * from a place the traveller added, so it draws the hotel a second time. + */ + createAssignment(dayId: string | number, placeId: unknown, notes?: string | null, opts: { accommodationId?: number } = {}) { const result = this.dbs.transaction(() => { const maxOrder = this.dbs.get<{ max: number | null }>('SELECT MAX(order_index) as max FROM day_assignments WHERE day_id = ?', dayId)!; const orderIndex = (maxOrder.max !== null ? maxOrder.max : -1) + 1; return this.dbs.run( - 'INSERT INTO day_assignments (day_id, place_id, order_index, notes) VALUES (?, ?, ?, ?)', - dayId, placeId, orderIndex, notes || null + 'INSERT INTO day_assignments (day_id, place_id, order_index, notes, accommodation_id) VALUES (?, ?, ?, ?, ?)', + dayId, placeId, orderIndex, notes || null, opts.accommodationId ?? null ); }); diff --git a/server/src/nest/common/rowShape.ts b/server/src/nest/common/rowShape.ts index 800f2c80a..4e0b36fdf 100644 --- a/server/src/nest/common/rowShape.ts +++ b/server/src/nest/common/rowShape.ts @@ -21,6 +21,10 @@ export function formatAssignmentWithPlace(a: AssignmentRow, tags: Partial[] end_day: a.end_day === 1, leg_transport_mode: a.leg_transport_mode ?? null, incoming_leg_transport_mode: a.incoming_leg_transport_mode ?? null, + // Which booking put this stop here, if a booking did. The day list has nothing + // else to tell it from a place the traveller added, and it must not draw the + // hotel a second time under the overnight block that already names it. + accommodation_id: a.accommodation_id ?? null, participants: participants || [], created_at: a.created_at, place: { diff --git a/server/src/nest/days/days.service.ts b/server/src/nest/days/days.service.ts index 625720bca..e26398746 100644 --- a/server/src/nest/days/days.service.ts +++ b/server/src/nest/days/days.service.ts @@ -114,6 +114,9 @@ export class DaysService { order_index: a.order_index, end_day: a.end_day === 1, notes: a.notes, + // Repeated here for the same reason stop_type is: the day list hides the stop + // a booking wrote, and this copy is one of the paths that feed it. + accommodation_id: a.accommodation_id ?? null, created_at: a.created_at, place: { id: a.place_id, diff --git a/server/src/nest/share/share.service.ts b/server/src/nest/share/share.service.ts index 9b36e992b..87d8851ef 100644 --- a/server/src/nest/share/share.service.ts +++ b/server/src/nest/share/share.service.ts @@ -316,6 +316,9 @@ export class ShareService { if (!byDay[a.day_id]) byDay[a.day_id] = []; byDay[a.day_id].push({ id: a.id, day_id: a.day_id, order_index: a.order_index, notes: a.notes, + // The shared page shows the booking as its own chip on the day, so it needs + // to know which stop is that booking and leave it out of the list. + accommodation_id: a.accommodation_id ?? null, place: { id: a.place_id, name: a.place_name, description: a.place_description, lat: a.lat, lng: a.lng, address: a.address, category_id: a.category_id, diff --git a/server/src/types.ts b/server/src/types.ts index 6b566c5fa..fedda2ffc 100644 --- a/server/src/types.ts +++ b/server/src/types.ts @@ -120,6 +120,8 @@ export interface DayAssignment { end_day?: number; leg_transport_mode?: string | null; incoming_leg_transport_mode?: string | null; + /** The lodging booking that put this stop on the day, when one did. */ + accommodation_id?: number | null; created_at?: string; } diff --git a/server/tests/unit/nest/accommodations.service.test.ts b/server/tests/unit/nest/accommodations.service.test.ts index c71818bf2..62cf19052 100644 --- a/server/tests/unit/nest/accommodations.service.test.ts +++ b/server/tests/unit/nest/accommodations.service.test.ts @@ -607,6 +607,11 @@ describe('the day stop a booking implies', () => { expect(stops).toHaveLength(1); expect(stops[0]).toMatchObject({ place_id: place.id, accommodation_id: accommodation.id }); expect(mirror.created).toMatchObject({ id: stops[0].id, day_id: day.id, place_id: place.id }); + // On the answer, not only in the table. This copy is what the client that booked + // the night puts in its store, and accommodation_id is the only thing telling the + // day list this row is the booking it already shows in the header. Without it the + // hotel appears a second time among the day's places until the next reload. + 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', () => {