From 875f87b885005ebb072f3011f5ea725bfb97f316 Mon Sep 17 00:00:00 2001 From: Maurice Date: Fri, 21 Aug 2026 13:59:06 +0200 Subject: [PATCH] fix(planner): open the booking a place is linked to MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The place inspector summarises the booking attached to the day assignment and then went nowhere: the strip is a bare div with no handler, no role and no tabIndex, and PlaceInspector was never handed an editor callback in the first place. Reached from a map marker, that strip is the only route a restaurant or tour booking has — only transports with two positioned endpoints get map pins of their own — so its fields and its attachments were unreachable. The identical lookup in the day sidebar has always carried a pencil that picks the editor by type. The inspector now takes the same onEditTransport / onEditReservation pair and does the same: a ferry, a flight, a train opens the transport form, everything else the reservation modal. That split matters — ReservationModal has no transport entry in its type list, so a ferry sent through it would have its type re-picked on save. Each handler arrives as undefined when the user lacks the matching right, which is what keeps the strip a read-only summary instead of turning it into a button that does nothing. Gating inside the handler would have re-shipped the reported symptom with a label claiming otherwise. The phone place sheet never rendered the strip at all, so the same booking was just as unreachable there, only invisibly. It shows it now and opens the same two editors, closing itself first — it hangs off the place selection rather than the sheet stack, so closeSheet() would leave the editor stacked on top of it. Closes #2012 --- .../Planner/PlaceInspector.test.tsx | 97 +++++++++++++++++++ .../src/components/Planner/PlaceInspector.tsx | 32 +++++- .../screens/trip/sheets/MPlaceSheet.tsx | 55 ++++++++++- client/src/pages/TripPlannerPage.tsx | 20 ++++ .../unit/mobile/trip/MPlaceSheet.test.tsx | 80 +++++++++++++++ 5 files changed, 279 insertions(+), 5 deletions(-) diff --git a/client/src/components/Planner/PlaceInspector.test.tsx b/client/src/components/Planner/PlaceInspector.test.tsx index 2288cb010..75fb0a8eb 100644 --- a/client/src/components/Planner/PlaceInspector.test.tsx +++ b/client/src/components/Planner/PlaceInspector.test.tsx @@ -425,6 +425,103 @@ describe('PlaceInspector', () => { expect(screen.getByText('Museum Ticket')).toBeTruthy(); }); + 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); + const assignmentInDay = [{ id: 99, place, day_id: 1, place_id: place.id, order_index: 0, notes: null }]; + render( + + ); + const strip = screen.getByText('Museum Ticket').closest('[role="button"]') as HTMLElement; + expect(strip).toBeTruthy(); + fireEvent.click(strip); + expect(onEditReservation).toHaveBeenCalledWith(reservation); + }); + + it('FE-PLANNER-INSPECTOR-030e: a transport booking goes to the transport editor (#2012)', () => { + const onEditTransport = vi.fn(); + const onEditReservation = vi.fn(); + // A ferry is a transport, and ReservationModal has no transport type to hold it. + const reservation = buildReservation({ title: 'Ferry to Corfu', status: 'pending', type: 'ferry', assignment_id: 99 } as any); + const assignmentInDay = [{ id: 99, place, day_id: 1, place_id: place.id, order_index: 0, notes: null }]; + render( + + ); + fireEvent.click(screen.getByText('Ferry to Corfu').closest('[role="button"]') as HTMLElement); + expect(onEditTransport).toHaveBeenCalledWith(reservation); + expect(onEditReservation).not.toHaveBeenCalled(); + }); + + it('FE-PLANNER-INSPECTOR-030f: no transport handler means no affordance on a transport (#2012)', () => { + // A member who may edit bookings but not days must not get a button that no-ops. + const reservation = buildReservation({ title: 'Ferry to Corfu', status: 'pending', type: 'ferry', assignment_id: 99 } as any); + const assignmentInDay = [{ id: 99, place, day_id: 1, place_id: place.id, order_index: 0, notes: null }]; + render( + + ); + expect(screen.getByText('Ferry to Corfu').closest('[role="button"]')).toBeNull(); + }); + + it('FE-PLANNER-INSPECTOR-030c: Enter and Space open it too (#2012)', () => { + const onEditReservation = vi.fn(); + const reservation = buildReservation({ title: 'Museum Ticket', status: 'confirmed', assignment_id: 99 } as any); + const assignmentInDay = [{ id: 99, place, day_id: 1, place_id: place.id, order_index: 0, notes: null }]; + render( + + ); + const strip = screen.getByText('Museum Ticket').closest('[role="button"]') as HTMLElement; + expect(strip.getAttribute('tabindex')).toBe('0'); + fireEvent.keyDown(strip, { key: 'Enter' }); + fireEvent.keyDown(strip, { key: ' ' }); + expect(onEditReservation).toHaveBeenCalledTimes(2); + }); + + it('FE-PLANNER-INSPECTOR-030d: without a handler the strip stays a read-only summary', () => { + const reservation = buildReservation({ title: 'Museum Ticket', status: 'confirmed', assignment_id: 99 } as any); + const assignmentInDay = [{ id: 99, place, day_id: 1, place_id: place.id, order_index: 0, notes: null }]; + render( + + ); + // A viewer with no edit right gets no target, and no misleading pointer. + expect(screen.getByText('Museum Ticket').closest('[role="button"]')).toBeNull(); + expect(screen.getByText('Museum Ticket')).toBeTruthy(); + }); + // ── Participants ─────────────────────────────────────────────────────────── it('FE-PLANNER-INSPECTOR-031: participants section shown when tripMembers > 1 and selectedAssignmentId is set', () => { diff --git a/client/src/components/Planner/PlaceInspector.tsx b/client/src/components/Planner/PlaceInspector.tsx index e89ab380f..663e5a9db 100644 --- a/client/src/components/Planner/PlaceInspector.tsx +++ b/client/src/components/Planner/PlaceInspector.tsx @@ -30,6 +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 { NavigationMenu } from '../shared/NavigationMenu' import { resolveOpenNow, resolvePlaceTimeZone, placeWeekdayIndex } from './placeOpenState' import { convertHoursLine } from './placeHoursFormat' @@ -143,6 +144,11 @@ interface PlaceInspectorProps { selectedAssignmentId?: number | null assignments?: AssignmentsMap reservations?: Reservation[] + /** Editors for the linked booking, each omitted when the user lacks that right — + * a transport needs day_edit, anything else reservation_edit. Both absent leaves + * the strip the read-only summary it has always been (#2012). */ + onEditTransport?: (reservation: Reservation) => void + onEditReservation?: (reservation: Reservation) => void onClose: () => void onEdit?: () => void onDelete?: () => void @@ -168,7 +174,7 @@ interface PlaceInspectorProps { export default function PlaceInspector({ place, categories, mode = 'trip', days = [], selectedDayId = null, selectedAssignmentId = null, - assignments = {}, reservations = [], + assignments = {}, reservations = [], onEditTransport, onEditReservation, onClose, onEdit, onDelete, onAssignToDay, onRemoveAssignment, files = [], onFileUpload, tripMembers = [], onSetParticipants, onUpdatePlace, onUploadImage, onRate, leftWidth = 0, rightWidth = 0, @@ -426,7 +432,8 @@ export default function PlaceInspector({ {mode === 'trip' && ( + timeFormat={timeFormat} t={t} onSetParticipants={onSetParticipants} + onEditTransport={onEditTransport} onEditReservation={onEditReservation} /> )} {/* Opening hours + Files — side by side on desktop only if both exist */} @@ -814,7 +821,7 @@ function PlaceInspectorHeader({ openNow, place, category, t, editingName, nameIn } function PlaceReservationParticipants({ selectedAssignmentId, reservations, assignments, selectedDayId, - tripMembers, locale, timeFormat, t, onSetParticipants }: any) { + tripMembers, locale, timeFormat, t, onSetParticipants, onEditTransport, onEditReservation }: any) { return ( <> {(() => { @@ -830,8 +837,25 @@ function PlaceReservationParticipants({ selectedAssignmentId, reservations, assi {/* 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')} diff --git a/client/src/mobile/screens/trip/sheets/MPlaceSheet.tsx b/client/src/mobile/screens/trip/sheets/MPlaceSheet.tsx index d7e0360e5..f42764967 100644 --- a/client/src/mobile/screens/trip/sheets/MPlaceSheet.tsx +++ b/client/src/mobile/screens/trip/sheets/MPlaceSheet.tsx @@ -1,6 +1,6 @@ import { useMemo, useRef, useState } from 'react' import { - Bookmark, Camera, ExternalLink, Loader2, Map as MapIcon, Navigation, Paperclip, + Bookmark, Camera, ChevronRight, ExternalLink, Loader2, Map as MapIcon, Navigation, Paperclip, Pencil, Phone, Plus, Route, Trash2, Upload, X, } from 'lucide-react' import MSheet from '../../../components/MSheet' @@ -85,6 +85,35 @@ 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 + // 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 + // 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', + planner.trip, + ) + const openLinkedRes = () => { + if (!linkedRes || !canOpenLinkedRes) return + if (planner.TRANSPORT_TYPES.has(linkedRes.type)) { + planner.setEditingTransport(linkedRes) + planner.setTransportModalDayId(linkedRes.day_id ?? null) + planner.setTransportModalAutomated(false) + planner.setShowTransportModal(true) + } else { + planner.setEditingReservation(linkedRes) + planner.setShowReservationModal(true) + } + // This sheet hangs off the place selection, not the sheet stack, so its own + // close() is what dismisses it — otherwise the editor opens on top of it. + close() + } + const participants = assignmentInDay?.participants || [] const participantIds = participants.map(p => p.user_id) const allJoined = participants.length === 0 @@ -403,6 +432,30 @@ export default function MPlaceSheet({ planner, shell }: MTripSheetsProps) {
)} + {/* ── The booking attached to this stop (#2012) ── */} + {linkedRes && ( + <> + {t('reservations.title')} + + + )} + {/* ── Participants of the selected day's assignment ── */} {assignmentInDay && members.length > 1 && ( <> diff --git a/client/src/pages/TripPlannerPage.tsx b/client/src/pages/TripPlannerPage.tsx index e7fc38464..e0859d5ed 100644 --- a/client/src/pages/TripPlannerPage.tsx +++ b/client/src/pages/TripPlannerPage.tsx @@ -279,6 +279,22 @@ function TripPlannerPageDesktop(): React.ReactElement | null { // coarse — a hybrid laptop loads drag-drop-touch instead. useTouchDragBridge(isTouch && !isMobile) + // The place inspector's booking strip opens the editor the booking belongs to. + // Handed over as undefined when the right is missing, so the strip stays a + // read-only summary rather than a button that does nothing (#2012). + const openLinkedTransport = can('day_edit', trip) ? (reservation: Reservation) => { + setEditingTransport(reservation) + setTransportModalDayId(reservation.day_id ?? null) + setTransportModalAutomated(false) + setShowTransportModal(true) + setMobileSidebarOpen(null) + } : undefined + const openLinkedReservation = can('reservation_edit', trip) ? (reservation: Reservation) => { + setEditingReservation(reservation) + setShowReservationModal(true) + setMobileSidebarOpen(null) + } : undefined + const poi = usePoiExplore() const [glMap, setGlMap] = useState(null) const poiPillEnabled = useSettingsStore(s => s.settings.map_poi_pill_enabled) !== false @@ -594,6 +610,8 @@ function TripPlannerPageDesktop(): React.ReactElement | null { {selectedPlace && !isMobile && ( setSelectedPlaceId(null)}>
e.stopPropagation()}> = {}) { ratePlace: vi.fn().mockResolvedValue(undefined), }, can: vi.fn(() => true), + reservations: [] as unknown[], + TRANSPORT_TYPES: new Set(['flight', 'train', 'bus', 'car', 'taxi', 'bicycle', 'cruise', 'ferry', 'transit', 'transport_other']), + setEditingTransport: vi.fn(), + setTransportModalDayId: vi.fn(), + setTransportModalAutomated: vi.fn(), + setShowTransportModal: vi.fn(), + setEditingReservation: vi.fn(), + setShowReservationModal: vi.fn(), toast: { success: vi.fn(), error: vi.fn(), warning: vi.fn(), info: vi.fn() }, ...overrides, } as unknown as TripPlanner @@ -429,4 +437,76 @@ describe('MPlaceSheet', () => { expect(screen.getAllByRole('button', { name: /Automatic color/ })[0]).toHaveAttribute('aria-expanded', 'false') }) + + // ── The booking attached to this stop (#2012) ── + + const bookingPlanner = (res: Record) => makePlanner({ + selectedAssignmentId: 11, + reservations: [res], + }) + + it('FE-MOB-PLSH-027: shows the booking attached to the day assignment (#2012)', () => { + renderSheet(bookingPlanner({ id: 7, assignment_id: 11, title: 'Ferry to Corfu', status: 'pending', type: 'ferry' })) + + expect(screen.getByText('Ferry to Corfu')).toBeInTheDocument() + }) + + it('FE-MOB-PLSH-028: a transport booking opens the transport editor, not the reservation modal', () => { + const planner = bookingPlanner({ id: 7, assignment_id: 11, title: 'Ferry to Corfu', status: 'pending', type: 'ferry', day_id: 2 }) + renderSheet(planner) + + fireEvent.click(screen.getByRole('button', { name: 'Edit Reservation' })) + + expect(planner.setEditingTransport).toHaveBeenCalledWith(expect.objectContaining({ id: 7 })) + expect(planner.setTransportModalDayId).toHaveBeenCalledWith(2) + expect(planner.setShowTransportModal).toHaveBeenCalledWith(true) + expect(planner.setShowReservationModal).not.toHaveBeenCalled() + // and the place sheet gets out of the way + expect(planner.setSelectedPlaceId).toHaveBeenCalledWith(null) + }) + + it('FE-MOB-PLSH-029: a non-transport booking opens the reservation modal', () => { + const planner = bookingPlanner({ id: 8, assignment_id: 11, title: 'Museum tour', status: 'confirmed', type: 'activity' }) + renderSheet(planner) + + fireEvent.click(screen.getByRole('button', { name: 'Edit Reservation' })) + + expect(planner.setEditingReservation).toHaveBeenCalledWith(expect.objectContaining({ id: 8 })) + expect(planner.setShowReservationModal).toHaveBeenCalledWith(true) + expect(planner.setShowTransportModal).not.toHaveBeenCalled() + }) + + it('FE-MOB-PLSH-030: a viewer without edit rights sees the booking but gets no live button', () => { + const planner = makePlanner({ + selectedAssignmentId: 11, + reservations: [{ id: 7, assignment_id: 11, title: 'Ferry to Corfu', status: 'pending', type: 'ferry' }], + can: vi.fn(() => false), + }) + renderSheet(planner) + + // Still readable — just not a control that would do nothing when pressed. + expect(screen.getByText('Ferry to Corfu')).toBeInTheDocument() + expect(screen.queryByRole('button', { name: 'Edit Reservation' })).not.toBeInTheDocument() + const row = screen.getByText('Ferry to Corfu').closest('button') as HTMLButtonElement + expect(row.disabled).toBe(true) + }) + + it('FE-MOB-PLSH-030b: day_edit alone is not enough for a plain booking, and vice versa', () => { + // A ferry needs day_edit; a member who only has reservation_edit gets no button. + const planner = makePlanner({ + selectedAssignmentId: 11, + reservations: [{ id: 7, assignment_id: 11, title: 'Ferry to Corfu', status: 'pending', type: 'ferry' }], + can: vi.fn((perm: string) => perm === 'reservation_edit'), + }) + renderSheet(planner) + + const row = screen.getByText('Ferry to Corfu').closest('button') as HTMLButtonElement + expect(row.disabled).toBe(true) + }) + + it('FE-MOB-PLSH-031: a booking on another assignment is not shown here', () => { + renderSheet(bookingPlanner({ id: 9, assignment_id: 999, title: 'Someone elses ferry', status: 'pending', type: 'ferry' })) + + expect(screen.queryByText('Someone elses ferry')).not.toBeInTheDocument() + }) })