fix(planner): open the booking a place is linked to

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
This commit is contained in:
Maurice
2026-08-21 13:59:06 +02:00
committed by Maurice
parent a23a81f35b
commit 875f87b885
5 changed files with 279 additions and 5 deletions
@@ -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(
<PlaceInspector
{...defaultProps}
selectedDayId={1}
selectedAssignmentId={99}
assignments={{ '1': assignmentInDay }}
reservations={[reservation]}
onEditReservation={onEditReservation}
/>
);
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(
<PlaceInspector
{...defaultProps}
selectedDayId={1}
selectedAssignmentId={99}
assignments={{ '1': assignmentInDay }}
reservations={[reservation]}
onEditTransport={onEditTransport}
onEditReservation={onEditReservation}
/>
);
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(
<PlaceInspector
{...defaultProps}
selectedDayId={1}
selectedAssignmentId={99}
assignments={{ '1': assignmentInDay }}
reservations={[reservation]}
onEditReservation={vi.fn()}
/>
);
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(
<PlaceInspector
{...defaultProps}
selectedDayId={1}
selectedAssignmentId={99}
assignments={{ '1': assignmentInDay }}
reservations={[reservation]}
onEditReservation={onEditReservation}
/>
);
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(
<PlaceInspector
{...defaultProps}
selectedDayId={1}
selectedAssignmentId={99}
assignments={{ '1': assignmentInDay }}
reservations={[reservation]}
/>
);
// 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', () => {
@@ -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' && (
<PlaceReservationParticipants selectedAssignmentId={selectedAssignmentId} reservations={reservations}
assignments={assignments} selectedDayId={selectedDayId} tripMembers={tripMembers} locale={locale}
timeFormat={timeFormat} t={t} onSetParticipants={onSetParticipants} />
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 (
<div style={{ borderRadius: 12, overflow: 'hidden', border: `1px solid ${confirmed ? 'rgba(22,163,74,0.2)' : 'rgba(217,119,6,0.2)'}` }}>
<div
role={open ? 'button' : undefined}
aria-label={open ? t('inspector.editRes') : undefined}
tabIndex={open ? 0 : undefined}
onClick={open}
onKeyDown={open ? (e: React.KeyboardEvent) => {
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' }}
>
<div className={confirmed ? 'bg-[rgba(22,163,74,0.08)]' : 'bg-[rgba(217,119,6,0.08)]'} style={{ display: 'flex', alignItems: 'center', gap: 8, padding: '6px 10px' }}>
<div className={confirmed ? 'bg-[#16a34a]' : 'bg-[#d97706]'} style={{ width: 6, height: 6, borderRadius: '50%', flexShrink: 0 }} />
<span className={confirmed ? 'text-[#16a34a]' : 'text-[#d97706]'} style={{ fontSize: 'calc(10px * var(--fs-scale-caption, 1))', fontWeight: 700 }}>{confirmed ? t('reservations.confirmed') : t('reservations.pending')}</span>
@@ -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) {
</div>
)}
{/* ── The booking attached to this stop (#2012) ── */}
{linkedRes && (
<>
<Eyebrow className="mb-[6px] mt-3">{t('reservations.title')}</Eyebrow>
<button
type="button"
onClick={openLinkedRes}
disabled={!canOpenLinkedRes}
aria-label={canOpenLinkedRes ? t('inspector.editRes') : undefined}
className={`flex w-full items-center gap-2 rounded-[14px] px-3 py-[10px] text-left ${INNER_CLS}`}
>
<span
className="h-2 w-2 flex-none rounded-full"
style={{ background: linkedRes.status === 'confirmed' ? 'var(--m-st-confirmed)' : 'var(--m-st-pending)' }}
/>
<span className="min-w-0 flex-1 truncate text-[0.8125rem] font-semibold">{linkedRes.title}</span>
<span className="flex-none font-geist text-[0.65625rem] text-m-muted">
{linkedRes.status === 'confirmed' ? t('reservations.confirmed') : t('reservations.pending')}
</span>
{canOpenLinkedRes && <ChevronRight size={14} strokeWidth={2} className="flex-none text-m-faint" />}
</button>
</>
)}
{/* ── Participants of the selected day's assignment ── */}
{assignmentInDay && members.length > 1 && (
<>
+20
View File
@@ -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<CompassMap | null>(null)
const poiPillEnabled = useSettingsStore(s => s.settings.map_poi_pill_enabled) !== false
@@ -594,6 +610,8 @@ function TripPlannerPageDesktop(): React.ReactElement | null {
{selectedPlace && !isMobile && (
<PlaceInspector
onEditTransport={openLinkedTransport}
onEditReservation={openLinkedReservation}
place={selectedPlace}
categories={categories}
days={days}
@@ -634,6 +652,8 @@ function TripPlannerPageDesktop(): React.ReactElement | null {
<div className="bg-[rgba(0,0,0,0.3)]" style={{ position: 'fixed', inset: 0, zIndex: 9999, display: 'flex', alignItems: 'flex-end', justifyContent: 'center', paddingBottom: 'var(--bottom-nav-h)' }} onClick={() => setSelectedPlaceId(null)}>
<div style={{ width: '100%', maxHeight: '85vh' }} onClick={e => e.stopPropagation()}>
<PlaceInspector
onEditTransport={openLinkedTransport}
onEditReservation={openLinkedReservation}
place={selectedPlace}
categories={categories}
days={days}
@@ -105,6 +105,14 @@ function makePlanner(overrides: Record<string, unknown> = {}) {
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<string, unknown>) => 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()
})
})