From faa27e36080a947d1df834c3593e2aae79618b71 Mon Sep 17 00:00:00 2001 From: Maurice Date: Wed, 2 Sep 2026 19:58:13 +0200 Subject: [PATCH] fix(planner): a place shows every booking linked to it, not just the first (#2201) Four render sites resolved the day assignment's booking with find(), while the booking dialog happily links several to the same stop. A shared selector in dayMerge returns them all, time-sorted and stable, and the desktop plan tab, the place inspector, the mobile timeline and the mobile place sheet render the full list; the single-booking case is unchanged pixel for pixel. --- .../Planner/DayPlanSidebar.test.tsx | 28 +++ .../src/components/Planner/DayPlanSidebar.tsx | 166 +++++++++-------- .../Planner/PlaceInspector.test.tsx | 27 +++ .../src/components/Planner/PlaceInspector.tsx | 173 +++++++++--------- .../screens/trip/plan/MPlanTimeline.tsx | 2 +- .../screens/trip/plan/MPlanTimelineRows.tsx | 29 ++- .../trip/plan/planTimelineModel.test.ts | 50 +++++ .../screens/trip/plan/planTimelineModel.ts | 9 +- .../trip/plan/useMPlanDragReorder.test.ts | 2 +- .../screens/trip/sheets/MPlaceSheet.tsx | 71 +++---- client/src/utils/dayMerge.test.ts | 37 +++- client/src/utils/dayMerge.ts | 30 +++ .../unit/mobile/trip/MPlanTimeline.test.tsx | 8 +- .../mobile/trip/MPlanTimelineRows.test.tsx | 6 +- .../mobile/trip/planTimelineModel.test.ts | 4 +- 15 files changed, 425 insertions(+), 217 deletions(-) create mode 100644 client/src/mobile/screens/trip/plan/planTimelineModel.test.ts diff --git a/client/src/components/Planner/DayPlanSidebar.test.tsx b/client/src/components/Planner/DayPlanSidebar.test.tsx index abd650b64..2ec0d95e0 100644 --- a/client/src/components/Planner/DayPlanSidebar.test.tsx +++ b/client/src/components/Planner/DayPlanSidebar.test.tsx @@ -3529,6 +3529,34 @@ describe('DayPlanSidebar', () => { expect(screen.getByText(/Reservation pending/)).toBeInTheDocument() }) + it('FE-PLANNER-DAYPLAN-163b: every booking on the stop gets its own chip line (#2201)', async () => { + const user = userEvent.setup() + const place = buildPlace({ id: 1, name: 'Zoo' }) + const day = buildDay({ id: 10, date: '2025-06-01', title: 'Day 1' }) + const a = buildAssignment({ id: 11, day_id: 10, order_index: 0, place }) + const parking = buildReservation({ + id: 530, type: 'parking', title: 'Parking pass', status: 'confirmed', assignment_id: 11, + reservation_time: '2025-06-01T09:00:00', reservation_end_time: '2025-06-01T09:30:00', + } as any) + const tickets = buildReservation({ + id: 531, type: 'activity', title: 'Zoo tickets', status: 'pending', assignment_id: 11, + reservation_time: '2025-06-01T10:15:00', + } as any) + const onEditReservation = vi.fn() + render() + expect(screen.getByText('09:00 – 09:30')).toBeInTheDocument() + expect(screen.getByText('10:15')).toBeInTheDocument() + expect(screen.getByText(/Reservation confirmed/)).toBeInTheDocument() + expect(screen.getByText(/Reservation pending/)).toBeInTheDocument() + // Earliest first, so the first pencil belongs to the parking pass. + await user.click(screen.getAllByTitle('Edit')[0]) + expect(onEditReservation).toHaveBeenCalledWith(parking) + }) + it('FE-PLANNER-DAYPLAN-164: travellers on a place row are shown as avatars with an overflow count', () => { const place = buildPlace({ id: 1, name: 'Group visit' }) const day = buildDay({ id: 10, date: '2025-06-01', title: 'Day 1' }) diff --git a/client/src/components/Planner/DayPlanSidebar.tsx b/client/src/components/Planner/DayPlanSidebar.tsx index e6cd725a7..cd0ac3017 100644 --- a/client/src/components/Planner/DayPlanSidebar.tsx +++ b/client/src/components/Planner/DayPlanSidebar.tsx @@ -31,6 +31,7 @@ import { isDayInAccommodationRange, getAccommodationAnchors, getDayBookendHotels import { TRANSPORT_TYPES, parseTimeToMinutes, getSpanPhase, hidesOnMiddleDay, getDisplayTimeForDay, getTransportRouteEndpoints, getTransportForDay as _getTransportForDay, getMergedItems as _getMergedItems, isCarrierTransport, hasCarrierEndpointOnDay, + getAssignmentReservations, type MergedItem, } from '../../utils/dayMerge' import { withinDriveRange } from '../../utils/geo' @@ -2141,86 +2142,97 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar(props: DayPlanSidebarP )} {(() => { - const res = reservations.find(r => r.assignment_id === assignment.id) - if (!res) return null - const confirmed = res.status === 'confirmed' - const hasEndpoints = onToggleConnection && (res.endpoints || []).length >= 2 - const active = hasEndpoints ? visibleConnectionIds.includes(res.id) : false - // The status, the time and the flight/train number used to sit in one - // pill strung together on a middle dot, which read as one run-on - // sentence. They are separate chips now: same tint so they still - // belong together, own outline so the eye can take them one at a time. - const RI = RES_ICONS[res.type] || Ticket - const tint = confirmed ? 'bg-[rgba(22,163,74,0.1)] text-[#16a34a]' : 'bg-[rgba(217,119,6,0.1)] text-[#d97706]' - const chip: React.CSSProperties = { - display: 'inline-flex', alignItems: 'center', gap: 3, - padding: '1px 6px', borderRadius: 5, - fontSize: 'calc(9px * var(--fs-scale-caption, 1))', fontWeight: 600, - whiteSpace: 'nowrap', - } - const { time: st } = splitReservationDateTime(res.reservation_time) - const { time: et } = splitReservationDateTime(res.reservation_end_time) - const timeLabel = st || et - ? `${st ? formatTime(st, locale, timeFormat) : ''}${et ? ` – ${formatTime(et, locale, timeFormat)}` : ''}` - : '' - let meta: any = {} - try { meta = typeof res.metadata === 'string' ? JSON.parse(res.metadata || '{}') : (res.metadata || {}) } catch { meta = {} } - const carrierLabel = meta - ? (meta.airline && meta.flight_number ? `${meta.airline} ${meta.flight_number}` : meta.flight_number || meta.train_number || '') - : '' + const linked = getAssignmentReservations(reservations, assignment.id) + if (linked.length === 0) return null + // A stop can carry more than one booking (a parking pass and the + // tickets for the same zoo), and every one of them is kept out of + // the timeline, so this row is where they have to appear (#2201). + // Stacked, because the chip lines are inline-flex and would + // otherwise run together on one. return ( - // No wrapping: the time belongs to the status it qualifies, so the - // chips stay on one line even when the row gets narrow. -
-
- - {confirmed ? t('planner.resConfirmed') : t('planner.resPending')} -
- {timeLabel && {timeLabel}} - {carrierLabel && {carrierLabel}} - {hasEndpoints && ( - - )} - {canEditDays && (() => { - const isTransport = TRANSPORT_TYPES.has(res.type) - const handler = isTransport ? onEditTransport : onEditReservation - if (!handler) return null +
+ {linked.map(res => { + const confirmed = res.status === 'confirmed' + const hasEndpoints = onToggleConnection && (res.endpoints || []).length >= 2 + const active = hasEndpoints ? visibleConnectionIds.includes(res.id) : false + // The status, the time and the flight/train number used to sit in one + // pill strung together on a middle dot, which read as one run-on + // sentence. They are separate chips now: same tint so they still + // belong together, own outline so the eye can take them one at a time. + const RI = RES_ICONS[res.type] || Ticket + const tint = confirmed ? 'bg-[rgba(22,163,74,0.1)] text-[#16a34a]' : 'bg-[rgba(217,119,6,0.1)] text-[#d97706]' + const chip: React.CSSProperties = { + display: 'inline-flex', alignItems: 'center', gap: 3, + padding: '1px 6px', borderRadius: 5, + fontSize: 'calc(9px * var(--fs-scale-caption, 1))', fontWeight: 600, + whiteSpace: 'nowrap', + } + const { time: st } = splitReservationDateTime(res.reservation_time) + const { time: et } = splitReservationDateTime(res.reservation_end_time) + const timeLabel = st || et + ? `${st ? formatTime(st, locale, timeFormat) : ''}${et ? ` – ${formatTime(et, locale, timeFormat)}` : ''}` + : '' + let meta: any = {} + try { meta = typeof res.metadata === 'string' ? JSON.parse(res.metadata || '{}') : (res.metadata || {}) } catch { meta = {} } + const carrierLabel = meta + ? (meta.airline && meta.flight_number ? `${meta.airline} ${meta.flight_number}` : meta.flight_number || meta.train_number || '') + : '' return ( - + // No wrapping: the time belongs to the status it qualifies, so the + // chips stay on one line even when the row gets narrow. +
+
+ + {confirmed ? t('planner.resConfirmed') : t('planner.resPending')} +
+ {timeLabel && {timeLabel}} + {carrierLabel && {carrierLabel}} + {hasEndpoints && ( + + )} + {canEditDays && (() => { + const isTransport = TRANSPORT_TYPES.has(res.type) + const handler = isTransport ? onEditTransport : onEditReservation + if (!handler) return null + return ( + + ) + })()} +
) - })()} + })}
) })()} diff --git a/client/src/components/Planner/PlaceInspector.test.tsx b/client/src/components/Planner/PlaceInspector.test.tsx index 57c01818c..40e15538e 100644 --- a/client/src/components/Planner/PlaceInspector.test.tsx +++ b/client/src/components/Planner/PlaceInspector.test.tsx @@ -425,6 +425,33 @@ describe('PlaceInspector', () => { expect(screen.getByText('Museum Ticket')).toBeTruthy(); }); + it('FE-PLANNER-INSPECTOR-030g: every booking on the stop gets its own strip (#2201)', () => { + const onEditReservation = vi.fn(); + const parking = buildReservation({ + id: 530, title: 'Parking pass', status: 'confirmed', type: 'parking', assignment_id: 99, + reservation_time: '2025-06-01T09:00:00', + } as any); + const tickets = buildReservation({ + id: 531, title: 'Zoo tickets', status: 'pending', type: 'activity', assignment_id: 99, + reservation_time: '2025-06-01T10:15:00', + } as any); + const assignmentInDay = [{ id: 99, place, day_id: 1, place_id: place.id, order_index: 0, notes: null }]; + render( + + ); + expect(screen.getByText('Parking pass')).toBeTruthy(); + expect(screen.getByText('Zoo tickets')).toBeTruthy(); + fireEvent.click(screen.getByText('Zoo tickets').closest('[role="button"]') as HTMLElement); + expect(onEditReservation).toHaveBeenCalledWith(tickets); + }); + it('FE-PLANNER-INSPECTOR-030b: the linked reservation opens its editor (#2012)', () => { const onEditReservation = vi.fn(); const reservation = buildReservation({ title: 'Museum Ticket', status: 'confirmed', assignment_id: 99 } as any); diff --git a/client/src/components/Planner/PlaceInspector.tsx b/client/src/components/Planner/PlaceInspector.tsx index 81639752d..c895f25e1 100644 --- a/client/src/components/Planner/PlaceInspector.tsx +++ b/client/src/components/Planner/PlaceInspector.tsx @@ -30,7 +30,7 @@ import { splitReservationDateTime, formatTime, formatMoney } from '../../utils/f import { useTripStore } from '../../store/tripStore' import { formatDistance, formatElevation } from '../../utils/units' import { getNavigationTargets, openNavigationTarget } from './placeNavigation' -import { TRANSPORT_TYPES } from '../../utils/dayMerge' +import { TRANSPORT_TYPES, getAssignmentReservations } from '../../utils/dayMerge' import { NavigationMenu } from '../shared/NavigationMenu' import { resolveOpenNow, resolvePlaceTimeZone, placeWeekdayIndex } from './placeOpenState' import { convertHoursLine } from './placeHoursFormat' @@ -839,96 +839,101 @@ function PlaceReservationParticipants({ selectedAssignmentId, reservations, assi return ( <> {(() => { - const res = selectedAssignmentId ? reservations.find(r => r.assignment_id === selectedAssignmentId) : null + const linked = getAssignmentReservations(reservations, selectedAssignmentId) const assignment = selectedAssignmentId ? (assignments[String(selectedDayId)] || []).find(a => a.id === selectedAssignmentId) : null const currentParticipants = assignment?.participants || [] const participantIds = currentParticipants.map(p => p.user_id) const allJoined = currentParticipants.length === 0 const showParticipants = selectedAssignmentId && tripMembers.length > 1 - if (!res && !showParticipants) return null + if (linked.length === 0 && !showParticipants) return null return ( -
- {/* Reservation */} - {res && (() => { - const confirmed = res.status === 'confirmed' - // The strip summarised the booking but went nowhere, so its - // attachments and fields had no route from the map (#2012). - // A transport has its own form; picking by type is what the day - // sidebar does, and an absent handler means this user may not - // open this one — so the strip stays inert rather than lying. - const editor = TRANSPORT_TYPES.has(res.type) ? onEditTransport : onEditReservation - const open = editor ? () => editor(res) : undefined - return ( -
{ - if (e.key === 'Enter' || e.key === ' ') { e.preventDefault(); open() } - } : undefined} - title={open ? t('inspector.editRes') : undefined} - style={{ borderRadius: 12, overflow: 'hidden', border: `1px solid ${confirmed ? 'rgba(22,163,74,0.2)' : 'rgba(217,119,6,0.2)'}`, cursor: open ? 'pointer' : undefined, textAlign: 'left' }} - > -
-
- {confirmed ? t('reservations.confirmed') : t('reservations.pending')} - - {res.title} -
-
- {(() => { - const { date, time: startTime } = splitReservationDateTime(res.reservation_time) - const { time: endTime } = splitReservationDateTime(res.reservation_end_time) - return ( - <> - {date && ( -
-
{t('reservations.date')}
-
{new Date(date + 'T00:00:00Z').toLocaleDateString(locale, { weekday: 'short', day: 'numeric', month: 'short', timeZone: 'UTC' })}
-
- )} - {(startTime || endTime) && ( -
-
{t('reservations.time')}
-
- {startTime ? formatTime(startTime, locale, timeFormat) : ''} - {endTime ? ` – ${formatTime(endTime, locale, timeFormat)}` : ''} -
-
- )} - - ) - })()} - {res.confirmation_number && ( -
-
{t('reservations.confirmationCode')}
-
{res.confirmation_number}
+
0 && showParticipants ? 'grid-cols-1 sm:grid-cols-2' : 'grid-cols-1'} gap-2`}> + {/* Bookings pinned to this stop; several can share one (#2201) */} + {linked.length > 0 && ( +
+ {linked.map(res => { + const confirmed = res.status === 'confirmed' + // The strip summarised the booking but went nowhere, so its + // attachments and fields had no route from the map (#2012). + // A transport has its own form; picking by type is what the day + // sidebar does, and an absent handler means this user may not + // open this one, so the strip stays inert rather than lying. + const editor = TRANSPORT_TYPES.has(res.type) ? onEditTransport : onEditReservation + const open = editor ? () => editor(res) : undefined + return ( +
{ + if (e.key === 'Enter' || e.key === ' ') { e.preventDefault(); open() } + } : undefined} + title={open ? t('inspector.editRes') : undefined} + style={{ borderRadius: 12, overflow: 'hidden', border: `1px solid ${confirmed ? 'rgba(22,163,74,0.2)' : 'rgba(217,119,6,0.2)'}`, cursor: open ? 'pointer' : undefined, textAlign: 'left' }} + > +
+
+ {confirmed ? t('reservations.confirmed') : t('reservations.pending')} + + {res.title}
- )} -
- {res.notes &&
{res.notes}
} - {(() => { - const meta = typeof res.metadata === 'string' ? JSON.parse(res.metadata || '{}') : (res.metadata || {}) - if (!meta || Object.keys(meta).length === 0) return null - const parts: string[] = [] - if (meta.airline && meta.flight_number) parts.push(`${meta.airline} ${meta.flight_number}`) - else if (meta.flight_number) parts.push(meta.flight_number) - if (meta.departure_airport && meta.arrival_airport) parts.push(`${meta.departure_airport} → ${meta.arrival_airport}`) - if (meta.train_number) parts.push(meta.train_number) - if (meta.platform) parts.push(`Gl. ${meta.platform}`) - if (meta.check_in_time) parts.push(`Check-in ${meta.check_in_time}`) - if (meta.check_out_time) parts.push(`Check-out ${meta.check_out_time}`) - if (parts.length === 0) return null - return
{parts.join(' · ')}
- })()} -
- ) - })()} +
+ {(() => { + const { date, time: startTime } = splitReservationDateTime(res.reservation_time) + const { time: endTime } = splitReservationDateTime(res.reservation_end_time) + return ( + <> + {date && ( +
+
{t('reservations.date')}
+
{new Date(date + 'T00:00:00Z').toLocaleDateString(locale, { weekday: 'short', day: 'numeric', month: 'short', timeZone: 'UTC' })}
+
+ )} + {(startTime || endTime) && ( +
+
{t('reservations.time')}
+
+ {startTime ? formatTime(startTime, locale, timeFormat) : ''} + {endTime ? ` – ${formatTime(endTime, locale, timeFormat)}` : ''} +
+
+ )} + + ) + })()} + {res.confirmation_number && ( +
+
{t('reservations.confirmationCode')}
+
{res.confirmation_number}
+
+ )} +
+ {res.notes &&
{res.notes}
} + {(() => { + const meta = typeof res.metadata === 'string' ? JSON.parse(res.metadata || '{}') : (res.metadata || {}) + if (!meta || Object.keys(meta).length === 0) return null + const parts: string[] = [] + if (meta.airline && meta.flight_number) parts.push(`${meta.airline} ${meta.flight_number}`) + else if (meta.flight_number) parts.push(meta.flight_number) + if (meta.departure_airport && meta.arrival_airport) parts.push(`${meta.departure_airport} → ${meta.arrival_airport}`) + if (meta.train_number) parts.push(meta.train_number) + if (meta.platform) parts.push(`Gl. ${meta.platform}`) + if (meta.check_in_time) parts.push(`Check-in ${meta.check_in_time}`) + if (meta.check_out_time) parts.push(`Check-out ${meta.check_out_time}`) + if (parts.length === 0) return null + return
{parts.join(' · ')}
+ })()} +
+ ) + })} +
+ )} {/* Participants */} {showParticipants && ( diff --git a/client/src/mobile/screens/trip/plan/MPlanTimeline.tsx b/client/src/mobile/screens/trip/plan/MPlanTimeline.tsx index c096e3045..cb0c030e1 100644 --- a/client/src/mobile/screens/trip/plan/MPlanTimeline.tsx +++ b/client/src/mobile/screens/trip/plan/MPlanTimeline.tsx @@ -175,7 +175,7 @@ export default function MPlanTimeline({ planner, shell }: MPlanTimelineProps) { ({ + id: r.id, + text: [ + linkedReservations.length > 1 ? r.title : '', + r.status === 'confirmed' ? t('dayplan.confirmed') : t('dayplan.pendingRes'), + r.confirmation_number ? `#${r.confirmation_number}` : '', + ].filter(Boolean).join(' · '), + })) + const sub = bookingLines.length > 0 + ? bookingLines[0].text : place?.address || place?.description || '' return ( @@ -175,7 +183,7 @@ export function PlaceRow({ assignment, fullPlace, linkedRes, chrome, reorder, dr
{place?.name} - {linkedRes && ( + {bookingLines.length > 0 && ( {t('mobileTrip.resBadge')} @@ -188,6 +196,11 @@ export function PlaceRow({ assignment, fullPlace, linkedRes, chrome, reorder, dr {sub && {sub}}
)} + {bookingLines.slice(1).map(line => ( +
+ {line.text} +
+ ))} {assignment.notes && ( // Day-specific note on this stop (#2163) — mirror of the desktop // timeline's caption line, so the phone shows the note exists diff --git a/client/src/mobile/screens/trip/plan/planTimelineModel.test.ts b/client/src/mobile/screens/trip/plan/planTimelineModel.test.ts new file mode 100644 index 000000000..b9dad5b97 --- /dev/null +++ b/client/src/mobile/screens/trip/plan/planTimelineModel.test.ts @@ -0,0 +1,50 @@ +import { describe, expect, it } from 'vitest' +import { buildPlanRows } from './planTimelineModel' +import type { MergedItem } from '../../../../utils/dayMerge' +import type { Reservation } from '../../../../types' + +// FE-MOBILE-PLANROWS-001 to FE-MOBILE-PLANROWS-003 + +const zooStop = [ + { type: 'place', sortKey: 0, data: { id: 11, day_id: 5, order_index: 0, place: { id: 1, name: 'Zoo' } } }, +] as unknown as MergedItem[] + +describe('buildPlanRows', () => { + it('FE-MOBILE-PLANROWS-001: carries every booking linked to the stop, earliest first (#2201)', () => { + const reservations = [ + { id: 531, assignment_id: 11, title: 'Zoo tickets', reservation_time: '2025-06-01T10:15:00' }, + { id: 530, assignment_id: 11, title: 'Parking pass', reservation_time: '2025-06-01T09:00:00' }, + { id: 532, assignment_id: 12, title: 'Another stop', reservation_time: null }, + ] as unknown as Reservation[] + const rows = buildPlanRows({ merged: zooStop, reservations, routeSegments: [], dayId: 5 }) + expect(rows).toHaveLength(1) + const row = rows[0] + if (row.kind !== 'place') throw new Error('expected a place row') + expect(row.linkedReservations.map(r => r.id)).toEqual([530, 531]) + }) + + it('FE-MOBILE-PLANROWS-002: a stop without bookings gets an empty list', () => { + const reservations = [ + { id: 532, assignment_id: 12, title: 'Another stop', reservation_time: null }, + ] as unknown as Reservation[] + const rows = buildPlanRows({ merged: zooStop, reservations, routeSegments: [], dayId: 5 }) + const row = rows[0] + if (row.kind !== 'place') throw new Error('expected a place row') + expect(row.linkedReservations).toEqual([]) + }) + + it('FE-MOBILE-PLANROWS-003: an unlinked booking stays a transport row of its own', () => { + const merged = [ + ...zooStop, + { type: 'transport', sortKey: 1, data: { id: 540, type: 'taxi', day_id: 5, title: 'Cab' } }, + ] as unknown as MergedItem[] + const reservations = [ + { id: 540, assignment_id: null, type: 'taxi', title: 'Cab', reservation_time: null }, + ] as unknown as Reservation[] + const rows = buildPlanRows({ merged, reservations, routeSegments: [], dayId: 5 }) + expect(rows.map(r => r.kind)).toEqual(['place', 'transport']) + const place = rows[0] + if (place.kind !== 'place') throw new Error('expected a place row') + expect(place.linkedReservations).toEqual([]) + }) +}) diff --git a/client/src/mobile/screens/trip/plan/planTimelineModel.ts b/client/src/mobile/screens/trip/plan/planTimelineModel.ts index 253e8cac8..f810aa774 100644 --- a/client/src/mobile/screens/trip/plan/planTimelineModel.ts +++ b/client/src/mobile/screens/trip/plan/planTimelineModel.ts @@ -1,6 +1,6 @@ import { Cloud, CloudDrizzle, CloudLightning, CloudRain, CloudSnow, Sun, Wind } from 'lucide-react' import type { LucideIcon } from 'lucide-react' -import { getDisplayTimeForDay, getSpanPhase, hidesOnMiddleDay, parseTimeToMinutes } from '../../../../utils/dayMerge' +import { getAssignmentReservations, getDisplayTimeForDay, getSpanPhase, hidesOnMiddleDay, parseTimeToMinutes } from '../../../../utils/dayMerge' import { getDayBookendHotels, isDayInAccommodationRange } from '../../../../utils/dayOrder' import type { MergedItem } from '../../../../utils/dayMerge' import type { TransitLegDisplay } from '../../../../components/Planner/transitDisplay' @@ -24,7 +24,7 @@ export interface TransitMeta { } export type PlanRow = - | { key: string; kind: 'place'; item: MergedItem; assignment: Assignment; linkedRes: Reservation | null } + | { key: string; kind: 'place'; item: MergedItem; assignment: Assignment; linkedReservations: Reservation[] } | { key: string; kind: 'transport'; item: MergedItem; res: TransportEntry } | { key: string; kind: 'transit'; item: MergedItem; res: TransportEntry; transit: TransitMeta } | { key: string; kind: 'note'; item: MergedItem; note: DayNote } @@ -100,7 +100,10 @@ export function buildPlanRows(opts: { kind: 'place', item, assignment, - linkedRes: reservations.find(r => r.assignment_id === assignment.id) ?? null, + // All of them: a stop can carry a parking pass and the tickets for the same + // attraction, and getTransportForDay keeps every linked booking out of the + // timeline, so anything dropped here is gone from the plan tab (#2201). + linkedReservations: getAssignmentReservations(reservations, assignment.id), }) } else if (item.type === 'note') { const note = item.data as DayNote diff --git a/client/src/mobile/screens/trip/plan/useMPlanDragReorder.test.ts b/client/src/mobile/screens/trip/plan/useMPlanDragReorder.test.ts index a7412b40f..736cea1b4 100644 --- a/client/src/mobile/screens/trip/plan/useMPlanDragReorder.test.ts +++ b/client/src/mobile/screens/trip/plan/useMPlanDragReorder.test.ts @@ -11,7 +11,7 @@ const items = [ ] as unknown as MergedItem[] const rowFor = (i: number): PlanRow => - ({ key: `row-${i}`, kind: 'place', item: items[i], assignment: {}, linkedRes: null }) as unknown as PlanRow + ({ key: `row-${i}`, kind: 'place', item: items[i], assignment: {}, linkedReservations: [] }) as unknown as PlanRow const connRow = { key: 'conn-1', kind: 'conn', seg: {} } as unknown as PlanRow diff --git a/client/src/mobile/screens/trip/sheets/MPlaceSheet.tsx b/client/src/mobile/screens/trip/sheets/MPlaceSheet.tsx index 75423e8f3..968c214a1 100644 --- a/client/src/mobile/screens/trip/sheets/MPlaceSheet.tsx +++ b/client/src/mobile/screens/trip/sheets/MPlaceSheet.tsx @@ -21,7 +21,8 @@ import { safeHttpUrl } from '../../../../utils/safeUrl' import { openFile } from '../../../../utils/fileDownload' import { getNavigationTargets, openNavigationTarget } from '../../../../components/Planner/placeNavigation' import { NavigationMenu } from '../../../../components/shared/NavigationMenu' -import type { Assignment, Day, TripMember } from '../../../../types' +import { getAssignmentReservations } from '../../../../utils/dayMerge' +import type { Assignment, Day, Reservation, TripMember } from '../../../../types' import { ActionCircle, Eyebrow, INNER_CLS } from './MTripSheetUi' /** @@ -85,28 +86,27 @@ export default function MPlaceSheet({ planner, shell }: MTripSheetsProps) { ? ((planner.selectedAssignmentId ? dayAssignments.find(a => a.id === planner.selectedAssignmentId) : null) ?? dayAssignments.find(a => a.place?.id === place?.id)) : null - // The booking attached to that assignment. The desktop inspector shows this + // The bookings attached to that assignment. The desktop inspector shows this // strip; the phone sheet never did, so a booking reached from a map marker was - // just as unreachable here, only invisibly so (#2012). - const linkedRes = assignmentInDay - ? planner.reservations.find(r => r.assignment_id === assignmentInDay.id) ?? null - : null + // just as unreachable here, only invisibly so (#2012). All of them, because a + // stop can carry a parking pass next to its tickets (#2201). + const linkedReservations = getAssignmentReservations(planner.reservations, assignmentInDay?.id) // A ferry or a flight has its own form — the reservation modal cannot hold one. // Resolved up front so a user without the matching right gets no button at all, // rather than one that does nothing (#2012). - const canOpenLinkedRes = !!linkedRes && planner.can( - planner.TRANSPORT_TYPES.has(linkedRes.type) ? 'day_edit' : 'reservation_edit', + const canOpenRes = (res: Reservation) => planner.can( + planner.TRANSPORT_TYPES.has(res.type) ? 'day_edit' : 'reservation_edit', planner.trip, ) - const openLinkedRes = () => { - if (!linkedRes || !canOpenLinkedRes) return - if (planner.TRANSPORT_TYPES.has(linkedRes.type)) { - planner.setEditingTransport(linkedRes) - planner.setTransportModalDayId(linkedRes.day_id ?? null) + const openRes = (res: Reservation) => { + if (!canOpenRes(res)) return + if (planner.TRANSPORT_TYPES.has(res.type)) { + planner.setEditingTransport(res) + planner.setTransportModalDayId(res.day_id ?? null) planner.setTransportModalAutomated(false) planner.setShowTransportModal(true) } else { - planner.setEditingReservation(linkedRes) + planner.setEditingReservation(res) planner.setShowReservationModal(true) } // This sheet hangs off the place selection, not the sheet stack, so its own @@ -452,27 +452,32 @@ export default function MPlaceSheet({ planner, shell }: MTripSheetsProps) { )} - {/* ── The booking attached to this stop (#2012) ── */} - {linkedRes && ( + {/* ── The bookings attached to this stop (#2012, #2201) ── */} + {linkedReservations.length > 0 && ( <> {t('reservations.title')} - +
+ {linkedReservations.map(res => ( + + ))} +
)} diff --git a/client/src/utils/dayMerge.test.ts b/client/src/utils/dayMerge.test.ts index b16dcd022..8608badd6 100644 --- a/client/src/utils/dayMerge.test.ts +++ b/client/src/utils/dayMerge.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from 'vitest' -import { parseTimeToMinutes, getSpanPhase, hidesOnMiddleDay, getTransportRouteEndpoints, getDisplayTimeForDay, getTransportForDay, getMergedItems } from './dayMerge' +import { parseTimeToMinutes, getSpanPhase, hidesOnMiddleDay, getTransportRouteEndpoints, getDisplayTimeForDay, getTransportForDay, getAssignmentReservations, getMergedItems } from './dayMerge' describe('parseTimeToMinutes', () => { it('parses HH:MM string', () => { @@ -199,6 +199,41 @@ describe('getTransportForDay', () => { }) }) +describe('getAssignmentReservations', () => { + it('returns every booking pinned to the assignment, not just the first (#2201)', () => { + const reservations = [ + { id: 1, assignment_id: 42, reservation_time: '2025-06-01T10:00:00' }, + { id: 2, assignment_id: 42, reservation_time: '2025-06-01T09:00:00' }, + { id: 3, assignment_id: 7, reservation_time: '2025-06-01T08:00:00' }, + ] + expect(getAssignmentReservations(reservations, 42).map(r => r.id)).toEqual([2, 1]) + }) + + it('puts untimed bookings last and breaks ties on the id', () => { + const reservations = [ + { id: 5, assignment_id: 42, reservation_time: null }, + { id: 4, assignment_id: 42, reservation_time: null }, + { id: 6, assignment_id: 42, reservation_time: '2025-06-01T09:00:00' }, + ] + expect(getAssignmentReservations(reservations, 42).map(r => r.id)).toEqual([6, 4, 5]) + }) + + it('returns nothing without an assignment', () => { + const reservations = [{ id: 1, assignment_id: 42, reservation_time: null }] + expect(getAssignmentReservations(reservations, null)).toEqual([]) + expect(getAssignmentReservations(reservations, undefined)).toEqual([]) + }) + + it('leaves the caller array untouched', () => { + const reservations = [ + { id: 1, assignment_id: 42, reservation_time: '2025-06-01T10:00:00' }, + { id: 2, assignment_id: 42, reservation_time: '2025-06-01T09:00:00' }, + ] + getAssignmentReservations(reservations, 42) + expect(reservations.map(r => r.id)).toEqual([1, 2]) + }) +}) + describe('getMergedItems', () => { it('merges places and notes sorted by sortKey', () => { const dayAssignments = [ diff --git a/client/src/utils/dayMerge.ts b/client/src/utils/dayMerge.ts index 157f27ae6..430ad054b 100644 --- a/client/src/utils/dayMerge.ts +++ b/client/src/utils/dayMerge.ts @@ -235,6 +235,36 @@ export function getTransportForDay(opts: { }).flatMap(r => expandFlightLegsForDay(r, dayId, getDayOrder, days)) } +/** + * Every booking pinned to one day assignment. A stop can carry several (the parking + * pass and the tickets for the same zoo), and the exclusion above keeps all of them + * out of the timeline, so the place row is the only surface that can show them, and + * a `find()` there dropped the rest without trace (#2201). + * + * Earliest first, untimed last, id as the tiebreaker: the store's array order is + * newest-first for locally created bookings and load order otherwise, which would + * shuffle the same two bookings between a reload and a live update. + */ +export function getAssignmentReservations(reservations: T[], assignmentId: number | null | undefined): T[] { + if (!assignmentId) return [] + return reservations + .filter(r => r.assignment_id === assignmentId) + .sort((a, b) => { + const at = a.reservation_time || '' + const bt = b.reservation_time || '' + if (at !== bt) { + if (!at) return 1 + if (!bt) return -1 + return at < bt ? -1 : 1 + } + return a.id - b.id + }) +} + /** * Order items chronologically: anything with a time (a place's place_time, a * transport/leg display time, a timed note) sorts by that time. An item WITHOUT a diff --git a/client/tests/unit/mobile/trip/MPlanTimeline.test.tsx b/client/tests/unit/mobile/trip/MPlanTimeline.test.tsx index 3f7d6ec37..42ac160d7 100644 --- a/client/tests/unit/mobile/trip/MPlanTimeline.test.tsx +++ b/client/tests/unit/mobile/trip/MPlanTimeline.test.tsx @@ -73,12 +73,12 @@ const M_PLACE2: MergedItem = { type: 'place', sortKey: 4, data: PARK } const MERGED = [M_PLACE, M_FLIGHT, M_TRANSIT, M_NOTE, M_PLACE2] const ROWS: PlanRow[] = [ - { key: 'pl-11', kind: 'place', item: M_PLACE, assignment: MUSEUM, linkedRes: null }, + { key: 'pl-11', kind: 'place', item: M_PLACE, assignment: MUSEUM, linkedReservations: [] }, { key: 'conn-pl-11', kind: 'conn', seg: SEG, assignmentId: 11 }, { key: 'tr-21', kind: 'transport', item: M_FLIGHT, res: FLIGHT }, { key: 'tr-22', kind: 'transit', item: M_TRANSIT, res: TRANSIT_RES, transit: TRANSIT }, { key: 'note-41', kind: 'note', item: M_NOTE, note: NOTE }, - { key: 'pl-12', kind: 'place', item: M_PLACE2, assignment: PARK, linkedRes: null }, + { key: 'pl-12', kind: 'place', item: M_PLACE2, assignment: PARK, linkedReservations: [] }, { key: 'conn-orphan', kind: 'conn', seg: { ...SEG, mode: 'walking' } }, ] @@ -265,7 +265,7 @@ describe('MPlanTimeline', () => { it('FE-MOB-PLTL-011: a place row without a place still reports the assignment', () => { const orphan = { id: 13, day_id: 2, place_id: 103, order_index: 2, place: null } as unknown as Assignment - const rows: PlanRow[] = [{ key: 'pl-13', kind: 'place', item: M_PLACE, assignment: orphan, linkedRes: null }] + const rows: PlanRow[] = [{ key: 'pl-13', kind: 'place', item: M_PLACE, assignment: orphan, linkedReservations: [] }] const { planner, container } = renderTimeline({ rows }) fireEvent.click(container.querySelector('.cursor-pointer.items-center') as HTMLElement) @@ -411,7 +411,7 @@ describe('MPlanTimeline', () => { byPosition: { 2: { start: [{ pluginId: 'ev', id: 's1', dayId: 2, position: 'start', label: 'Morning prep', tone: 'default' }], end: [] } }, minutesByDay: { 2: 35 }, } - const rows: PlanRow[] = [{ key: 'pl-11', kind: 'place', item: M_PLACE, assignment: MUSEUM, linkedRes: null }] + const rows: PlanRow[] = [{ key: 'pl-11', kind: 'place', item: M_PLACE, assignment: MUSEUM, linkedReservations: [] }] renderTimeline({ day: undefined, rows }) expect(screen.queryByText('Charging stop')).not.toBeInTheDocument() diff --git a/client/tests/unit/mobile/trip/MPlanTimelineRows.test.tsx b/client/tests/unit/mobile/trip/MPlanTimelineRows.test.tsx index ce14effbd..7dfe25b34 100644 --- a/client/tests/unit/mobile/trip/MPlanTimelineRows.test.tsx +++ b/client/tests/unit/mobile/trip/MPlanTimelineRows.test.tsx @@ -82,7 +82,7 @@ describe('PlaceRow', () => { const props = { assignment: assignment(), fullPlace: undefined, - linkedRes: null, + linkedReservations: [], chrome: chrome(), reorder: REORDER, onOpen: vi.fn(), @@ -116,7 +116,7 @@ describe('PlaceRow', () => { it('FE-MOB-PLROW-007: a linked booking replaces the subtitle and adds the booking badge', () => { const linkedRes = { id: 31, status: 'confirmed', confirmation_number: 'X9K' } as unknown as Reservation - render() + render() expect(screen.getByText('dayplan.confirmed · #X9K')).toBeInTheDocument() expect(screen.getByText('mobileTrip.resBadge')).toBeInTheDocument() @@ -124,7 +124,7 @@ describe('PlaceRow', () => { it('FE-MOB-PLROW-008: a pending booking without a number shows just the status', () => { const linkedRes = { id: 31, status: 'pending', confirmation_number: null } as unknown as Reservation - render() + render() expect(screen.getByText('dayplan.pendingRes')).toBeInTheDocument() }) diff --git a/client/tests/unit/mobile/trip/planTimelineModel.test.ts b/client/tests/unit/mobile/trip/planTimelineModel.test.ts index 1ece1187c..f2d617cd6 100644 --- a/client/tests/unit/mobile/trip/planTimelineModel.test.ts +++ b/client/tests/unit/mobile/trip/planTimelineModel.test.ts @@ -166,8 +166,8 @@ describe('planTimelineModel — buildPlanRows', () => { merged: [placeItem(museum), placeItem(park)], reservations: [dinner], routeSegments: [], dayId: 2, }) const [first, second] = rows.filter(r => r.kind === 'place') - expect(first.kind === 'place' && first.linkedRes).toBe(dinner) - expect(second.kind === 'place' && second.linkedRes).toBeNull() + expect(first.kind === 'place' && first.linkedReservations[0]).toBe(dinner) + expect(second.kind === 'place' && second.linkedReservations.length === 0).toBe(true) }) it('FE-MOB-PTLM-014: keys a synthetic leg row by its leg index', () => {