fix(bookings): the phone keeps a booking's stop and shows the files it already has (#2216, #2217)

Editing a booking on the phone erased the stop it was linked to. The sheet
seeded its assignment from the create-for-a-stop flag, which is null on an
edit, and saved that straight back over a link the user had made on the
desktop; the server writes the field whenever the key is present. It now
seeds from the booking being edited, and the picker the desktop dialog has
always had sits in the sheet too, so a stop can be seen, changed and cleared
from a phone. The option list moved to its own module rather than being
copied.

Two more of the same shape, found on the way. The endpoint set was sent as an
empty list on every save, and the server swaps the whole set when that key is
present, so editing a transit booking dropped its stations; neither form
edits endpoints, so an edit no longer sends the key at all. And the sheet's
Files block listed only what was picked in this session, so a file uploaded
from the desktop was invisible on the phone. It lists what is attached now,
resolved the way the desktop resolves it.

A place also shows the files of the bookings that sit on it, on the phone and
on the desktop: a confirmation lives on the booking, and the place card was
the one screen that could not reach it.
This commit is contained in:
Maurice
2026-09-03 15:51:13 +02:00
parent 9228ef2bce
commit 07e4a558e6
8 changed files with 289 additions and 34 deletions
@@ -12,6 +12,7 @@ import PlaceAvatarUpload from '../shared/PlaceAvatarUpload'
import PlaceRating from '../shared/StarRating'
import TrackColorPicker from '../shared/TrackColorPicker'
import { resolveTrackColor, inheritedTrackColor } from '../Map/trackColors'
import { filesForPlace } from '../../utils/placeFiles'
import GuestBadge from '../shared/GuestBadge'
import StatusBadge from '../Collections/StatusBadge'
import { mapsApi, pluginsApi } from '../../api/client'
@@ -337,7 +338,8 @@ export default function PlaceInspector({
const selectedDay = days?.find(d => d.id === selectedDayId)
const weekdayIndex = getWeekdayIndex(selectedDay?.date, placeTimeZone)
const placeFiles = (files || []).filter(f => String(f.place_id) === String(place.id) || (f.linked_place_ids || []).includes(place.id))
// Its own files plus the ones on the bookings that hang on it (#2217).
const placeFiles = filesForPlace(files, place.id, reservations, selectedAssignmentId != null ? [selectedAssignmentId] : [])
return (
<div
@@ -6,6 +6,7 @@ import { useTripStore } from '../../store/tripStore'
import { useAddonStore } from '../../store/addonStore'
import Modal from '../shared/Modal'
import CustomSelect from '../shared/CustomSelect'
import { buildAssignmentOptions } from './assignmentOptions'
import AddressInput from './AddressInput'
import { Hotel, Utensils, Ticket, FileText, Users, Paperclip, X, ExternalLink, Link2, ParkingSquare } from 'lucide-react'
import { useToast } from '../shared/Toast'
@@ -32,31 +33,6 @@ const TYPE_OPTIONS = [
{ value: 'other', labelKey: 'reservations.type.other', Icon: FileText },
]
function buildAssignmentOptions(days, assignments, t, locale) {
const options = []
for (const day of (days || [])) {
const da = (assignments?.[String(day.id)] || []).slice().sort((a, b) => a.order_index - b.order_index)
if (da.length === 0) continue
const dayLabel = day.title || t('dayplan.dayN', { n: day.day_number })
const dateStr = day.date ? ` · ${formatDate(day.date, locale)}` : ''
const groupLabel = `${dayLabel}${dateStr}`
options.push({ value: `_header_${day.id}`, label: groupLabel, disabled: true, isHeader: true })
for (let i = 0; i < da.length; i++) {
const place = da[i].place
if (!place) continue
const timeStr = place.place_time ? ` · ${place.place_time}${place.end_time ? ' – ' + place.end_time : ''}` : ''
options.push({
value: da[i].id,
label: ` ${i + 1}. ${place.name}${timeStr}`,
searchLabel: place.name,
groupLabel,
dayDate: day.date || null,
})
}
}
return options
}
interface ReservationModalProps {
isOpen: boolean
onClose: () => void
@@ -279,7 +255,10 @@ export function ReservationModal({ isOpen, onClose, onSave, reservation, days, p
// the picked trip place/activity directly on the reservation (#1353).
place_id: form.type === 'hotel' ? null : (form.place_id || null),
metadata: Object.keys(metadata).length > 0 ? metadata : null,
endpoints: [],
// Omitted on an edit: the server replaces the endpoint set whenever the
// key is present, and this form never edits endpoints, so sending an
// empty list would drop a transit booking's stations (#2216).
...(reservation?.id ? {} : { endpoints: [] }),
needs_review: false,
}
if (form.type === 'hotel' && (form.hotel_start_day || form.hotel_end_day)) {
@@ -0,0 +1,52 @@
import type { Assignment, Day, TranslationFn } from '../../types'
/** Day and month only, the shape the booking dialog has always shown here. */
function dayBadge(dateStr: string, locale?: string): string {
return new Date(dateStr + 'T00:00:00Z')
.toLocaleDateString(locale || undefined, { day: 'numeric', month: 'short', timeZone: 'UTC' })
}
export interface AssignmentOption {
value: number | string
label: string
searchLabel?: string
groupLabel?: string
dayDate?: string | null
disabled?: boolean
isHeader?: boolean
}
/**
* The day plan as a flat option list: a disabled header per day, then its stops
* in plan order. Shared by the desktop booking dialog and the phone sheet so a
* booking can be linked to the same stop from either (#2216).
*/
export function buildAssignmentOptions(
days: Day[] | undefined,
assignments: Record<string, Assignment[]> | undefined,
t: TranslationFn,
locale?: string,
): AssignmentOption[] {
const options: AssignmentOption[] = []
for (const day of (days || [])) {
const da = (assignments?.[String(day.id)] || []).slice().sort((a, b) => a.order_index - b.order_index)
if (da.length === 0) continue
const dayLabel = day.title || t('dayplan.dayN', { n: day.day_number })
const dateStr = day.date ? ` · ${dayBadge(day.date, locale)}` : ''
const groupLabel = `${dayLabel}${dateStr}`
options.push({ value: `_header_${day.id}`, label: groupLabel, disabled: true, isHeader: true })
for (let i = 0; i < da.length; i++) {
const place = da[i].place
if (!place) continue
const timeStr = place.place_time ? ` · ${place.place_time}${place.end_time ? ' – ' + place.end_time : ''}` : ''
options.push({
value: da[i].id,
label: ` ${i + 1}. ${place.name}${timeStr}`,
searchLabel: place.name,
groupLabel,
dayDate: day.date || null,
})
}
}
return options
}
@@ -19,6 +19,7 @@ import { resolveTrackColor, inheritedTrackColor } from '../../../../components/M
import { avatarSrc } from '../../../../utils/avatarSrc'
import { safeHttpUrl } from '../../../../utils/safeUrl'
import { openFile } from '../../../../utils/fileDownload'
import { filesForPlace } from '../../../../utils/placeFiles'
import { getNavigationTargets, openNavigationTarget } from '../../../../components/Planner/placeNavigation'
import { NavigationMenu } from '../../../../components/shared/NavigationMenu'
import { getAssignmentReservations } from '../../../../utils/dayMerge'
@@ -151,8 +152,12 @@ export default function MPlaceSheet({ planner, shell }: MTripSheetsProps) {
setParticipantPickerOpen(false)
}
const placeFiles = (planner.files || []).filter(f =>
!f.deleted_at && (String(f.place_id) === String(place?.id) || (f.linked_place_ids || []).includes(place?.id ?? -1)),
// Its own files plus the ones on the bookings that hang on it (#2217).
const placeFiles = filesForPlace(
planner.files,
place?.id,
planner.reservations,
assignmentInDay ? [assignmentInDay.id] : [],
)
const handleImagePick = async (e: React.ChangeEvent<HTMLInputElement>) => {
@@ -1,5 +1,5 @@
import { useEffect, useRef, useState } from 'react'
import { Check, FileText, Hotel, Link2, ParkingSquare, Plus, Ticket, Users, Utensils } from 'lucide-react'
import { useEffect, useMemo, useRef, useState } from 'react'
import { Check, ExternalLink, FileText, Hotel, Link2, ParkingSquare, Plus, Ticket, Users, Utensils } from 'lucide-react'
import MSheet from '../../../components/MSheet'
import { useAddonStore } from '../../../../store/addonStore'
import { useTranslation } from '../../../../i18n'
@@ -11,6 +11,8 @@ import CustomTimePicker from '../../../../components/shared/CustomTimePicker'
import { CustomDatePicker } from '../../../../components/shared/CustomDateTimePicker'
import { Eyebrow, FIELD_AREA_CLS, FIELD_CLS, FormSheetFooter, FormSheetHeader } from './PlSheetChrome'
import PlFileAttach from './PlFileAttach'
import { buildAssignmentOptions } from '../../../../components/Planner/assignmentOptions'
import { openFile } from '../../../../utils/fileDownload'
import GuestBadge from '../../../../components/shared/GuestBadge'
import { SPLIT_COLORS } from '../../../../components/Budget/BudgetPanel.constants'
import { useTripStore } from '../../../../store/tripStore'
@@ -57,6 +59,7 @@ export default function MReservationSheet({ planner, onOpenExpense }: MReservati
showReservationModal, setShowReservationModal,
editingReservation, setEditingReservation, reservationPrefill,
bookingForAssignmentId, setBookingForAssignmentId,
assignments, files,
importReviewActive, advanceImportReview,
handleSaveReservation, canUploadFiles, tripActions,
} = planner
@@ -67,6 +70,11 @@ export default function MReservationSheet({ planner, onOpenExpense }: MReservati
const [form, setForm] = useState(EMPTY)
const [pendingFiles, setPendingFiles] = useState<File[]>([])
const assignmentOptions = useMemo(
() => buildAssignmentOptions(days, assignments, t, locale),
[days, assignments, t, locale],
)
// Travelers assigned to this booking (#1517) — seeded from the editing
// reservation on open, persisted separately after the save resolves.
const [travelerIds, setTravelerIds] = useState<Set<number>>(new Set())
@@ -77,10 +85,15 @@ export default function MReservationSheet({ planner, onOpenExpense }: MReservati
const [snap, setSnap] = useState<{ res: typeof editingReservation; assignmentId: number | null }>(
{ res: null, assignmentId: null },
)
// The stop this booking hangs on. Seeded from the booking being edited, not
// only from the create-for-a-stop flag, because saving used to write the flag
// straight back and an edit therefore erased the link (#2216).
const [assignmentId, setAssignmentId] = useState<number | ''>('')
useEffect(() => {
if (!showReservationModal) return
setSnap({ res: editingReservation, assignmentId: bookingForAssignmentId ?? null })
setAssignmentId(bookingForAssignmentId ?? editingReservation?.assignment_id ?? '')
expenseIntentRef.current = false
setPendingFiles([])
setTravelerIds(new Set((editingReservation?.travelers || []).map(tv => tv.user_id)))
@@ -147,6 +160,16 @@ export default function MReservationSheet({ planner, onOpenExpense }: MReservati
}, [showReservationModal])
const res = snap.res
// Files already on this booking. The sheet used to list only the ones picked
// in this session, so an upload from the desktop was invisible here (#2217).
const attachedFiles = res?.id
? (files || []).filter(f =>
!f.deleted_at && (
String(f.reservation_id) === String(res.id) ||
(f.linked_reservation_ids || []).includes(res.id)
),
)
: []
const isHotel = form.type === 'hotel'
const set = (field: keyof typeof EMPTY, value: string | number) => setForm(prev => ({ ...prev, [field]: value }))
@@ -233,11 +256,15 @@ export default function MReservationSheet({ planner, onOpenExpense }: MReservati
location: isHotel ? form.hotel_address : form.location,
confirmation_number: form.confirmation_number,
notes: form.notes, url: form.url,
assignment_id: (isHotel && !form.accommodation_id) ? null : (snap.assignmentId || null),
assignment_id: (isHotel && !form.accommodation_id) ? null : (assignmentId || null),
accommodation_id: isHotel ? (form.accommodation_id || null) : null,
place_id: isHotel ? null : (form.place_id || null),
metadata: Object.keys(metadata).length > 0 ? metadata : null,
endpoints: [], needs_review: false,
// Omitted on an edit for the same reason as the desktop dialog: the
// server swaps the whole endpoint set when the key is present, and this
// sheet never edits endpoints (#2216).
...(snap.res?.id ? {} : { endpoints: [] }),
needs_review: false,
}
if (isHotel && (form.hotel_start_day || form.hotel_end_day)) {
saveData.create_accommodation = {
@@ -556,7 +583,46 @@ export default function MReservationSheet({ planner, onOpenExpense }: MReservati
</div>
)}
{/* LINK TO A STOP IN THE PLAN (#2216) */}
{!isHotel && assignmentOptions.length > 0 && (
<>
<Eyebrow className="mb-[6px] mt-3 uppercase">{t('reservations.linkAssignment')}</Eyebrow>
<CustomSelect
value={assignmentId}
onChange={value => {
setAssignmentId(value === '' ? '' : Number(value))
const opt = assignmentOptions.find(o => o.value === value)
// Same courtesy as the desktop dialog: an undated booking takes
// the day of the stop it was just linked to.
if (opt?.dayDate) setForm(prev => (prev.reservation_time ? prev : { ...prev, reservation_time: opt.dayDate! }))
}}
placeholder={t('reservations.pickAssignment')}
options={[{ value: '', label: t('reservations.noAssignment') }, ...assignmentOptions]}
searchable
size="sm"
/>
</>
)}
{/* FILES */}
{attachedFiles.length > 0 && (
<>
<Eyebrow className="mb-[6px] mt-3 uppercase">{t('files.title')}</Eyebrow>
<div className="flex flex-col gap-1">
{attachedFiles.map(f => (
<button
key={f.id}
type="button"
onClick={() => openFile(f.url, f.original_name)}
className="flex w-full items-center gap-2 rounded-[10px] bg-[color:var(--m-ic)] px-[10px] py-[7px] text-left"
>
<span className="min-w-0 flex-1 truncate text-[0.75rem] font-medium">{f.original_name}</span>
<ExternalLink size={11} strokeWidth={2} className="flex-none text-m-faint" />
</button>
))}
</div>
</>
)}
{canUploadFiles && (
<PlFileAttach
planner={planner}
+34
View File
@@ -0,0 +1,34 @@
import type { Reservation, TripFile } from '../types'
/**
* The files a place shows: its own, plus the ones attached to a booking that
* hangs on this place or on one of its stops in the plan. A confirmation lives
* on the booking, and the place card was the one screen that could not reach it
* (#2217).
*/
export function filesForPlace(
files: TripFile[] | undefined,
placeId: number | undefined,
reservations: Reservation[] | undefined,
assignmentIds: number[] = [],
): TripFile[] {
if (placeId == null) return []
const own = (files || []).filter(f =>
!f.deleted_at && (String(f.place_id) === String(placeId) || (f.linked_place_ids || []).includes(placeId)),
)
const bookingIds = new Set(
(reservations || [])
.filter(r => String(r.place_id) === String(placeId) || (r.assignment_id != null && assignmentIds.includes(r.assignment_id)))
.map(r => r.id),
)
if (bookingIds.size === 0) return own
const seen = new Set(own.map(f => f.id))
return [
...own,
...(files || []).filter(f => {
if (f.deleted_at || seen.has(f.id)) return false
if (f.reservation_id != null && bookingIds.has(Number(f.reservation_id))) return true
return (f.linked_reservation_ids || []).some(id => id != null && bookingIds.has(id))
}),
]
}
@@ -2,7 +2,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import MReservationSheet from '../../../../src/mobile/screens/trip/sheets/MReservationSheet'
import { useAddonStore } from '../../../../src/store/addonStore'
import { useTripStore } from '../../../../src/store/tripStore'
import type { Accommodation, Day, Place, Reservation, TripMember } from '../../../../src/types'
import type { Accommodation, Assignment, Day, Place, Reservation, TripMember } from '../../../../src/types'
import { buildPlanner } from '../../../helpers/mobileTrip'
import { resetAllStores } from '../../../helpers/store'
import { fireEvent, render, screen, waitFor } from '../../../helpers/render'
@@ -76,6 +76,11 @@ const ACCOMMODATIONS = [
{ id: 77, trip_id: 5, place_id: 101, start_day_id: 12, end_day_id: 13 },
] as unknown as Accommodation[]
const ASSIGNMENT = {
id: 11, day_id: 12, order_index: 0, place_id: 102,
place: { id: 102, name: 'Cafe Central', place_time: '19:30' },
} as unknown as Assignment
const MEMBERS = [
{ id: 7, username: 'Ana', avatar_url: '/uploads/ana.png' },
{ id: 8, username: 'bob', is_guest: true },
@@ -209,6 +214,61 @@ describe('MReservationSheet', () => {
})
})
it('FE-MOB-RESSH-054: editing a booking keeps the stop it was linked to (#2216)', async () => {
const linked = { ...DINNER, id: 71, assignment_id: 11, title: 'Zoo tickets' } as unknown as Reservation
const planner = makePlanner({ editingReservation: linked, assignments: { 12: [ASSIGNMENT] } })
setup(planner)
fireEvent.click(submitBtn())
await waitFor(() => expect(planner.handleSaveReservation).toHaveBeenCalled())
// Saving without touching the picker used to send null and erase the link.
expect(savedPayload(planner).assignment_id).toBe(11)
// And the endpoint set is left alone rather than replaced by an empty one.
expect(savedPayload(planner)).not.toHaveProperty('endpoints')
})
it('FE-MOB-RESSH-055: the picker shows the plan and can unlink the booking (#2216)', async () => {
const linked = { ...DINNER, id: 71, assignment_id: 11 } as unknown as Reservation
const planner = makePlanner({ editingReservation: linked, assignments: { 12: [ASSIGNMENT] } })
setup(planner)
expect(screen.getByText('reservations.linkAssignment')).toBeInTheDocument()
const picker = selects().find(s => s.getAttribute('data-value') === '11')
expect(picker).toBeTruthy()
pick(picker!, '')
fireEvent.click(submitBtn())
await waitFor(() => expect(planner.handleSaveReservation).toHaveBeenCalled())
expect(savedPayload(planner).assignment_id).toBeNull()
})
it('FE-MOB-RESSH-056: a booking created for a stop still saves that stop', async () => {
const planner = makePlanner({ bookingForAssignmentId: 11, assignments: { 12: [ASSIGNMENT] } })
setup(planner)
type(titleField(), 'Zoo tickets')
fireEvent.click(submitBtn())
await waitFor(() => expect(planner.handleSaveReservation).toHaveBeenCalled())
expect(savedPayload(planner).assignment_id).toBe(11)
// A create still declares the endpoint set, which is what the server expects.
expect(savedPayload(planner).endpoints).toEqual([])
})
it('FE-MOB-RESSH-057: the files already on the booking are listed, not just this session\'s picks (#2217)', () => {
const linked = { ...DINNER, id: 71 } as unknown as Reservation
const files = [
{ id: 4, trip_id: 5, reservation_id: 71, original_name: 'voucher.pdf', url: '/uploads/voucher.pdf', deleted_at: null },
{ id: 5, trip_id: 5, reservation_id: 99, original_name: 'other.pdf', url: '/uploads/other.pdf', deleted_at: null },
{ id: 6, trip_id: 5, linked_reservation_ids: [71], original_name: 'linked.pdf', url: '/uploads/linked.pdf', deleted_at: null },
]
setup(makePlanner({ editingReservation: linked, files }))
expect(screen.getByText('voucher.pdf')).toBeInTheDocument()
expect(screen.getByText('linked.pdf')).toBeInTheDocument()
expect(screen.queryByText('other.pdf')).not.toBeInTheDocument()
})
it('FE-MOB-RESSH-007: the type chips switch the form between event and hotel layout', () => {
setup()
expect(screen.getByLabelText('reservations.meta.pickPlace')).toBeInTheDocument()
@@ -0,0 +1,57 @@
import { describe, expect, it } from 'vitest'
import { filesForPlace } from '../../../src/utils/placeFiles'
import type { Reservation, TripFile } from '../../../src/types'
// FE-UTIL-PLACEFILES-001 to FE-UTIL-PLACEFILES-006
const file = (over: Partial<TripFile>): TripFile => ({
id: 1, trip_id: 5, original_name: 'a.pdf', url: '/uploads/a.pdf',
place_id: null, reservation_id: null, deleted_at: null,
...over,
} as unknown as TripFile)
const booking = (over: Partial<Reservation>): Reservation => ({
id: 90, trip_id: 5, title: 'Table', type: 'restaurant',
place_id: null, assignment_id: null,
...over,
} as unknown as Reservation)
describe('filesForPlace', () => {
it('FE-UTIL-PLACEFILES-001: keeps the files the place owns, by column and by link', () => {
const own = file({ id: 1, place_id: 7 })
const linked = file({ id: 2, linked_place_ids: [7] })
const other = file({ id: 3, place_id: 8 })
expect(filesForPlace([own, linked, other], 7, []).map(f => f.id)).toEqual([1, 2])
})
it('FE-UTIL-PLACEFILES-002: adds the files of a booking that sits on the place (#2217)', () => {
const files = [file({ id: 4, reservation_id: 90 })]
const bookings = [booking({ id: 90, place_id: 7 })]
expect(filesForPlace(files, 7, bookings).map(f => f.id)).toEqual([4])
})
it('FE-UTIL-PLACEFILES-003: reaches a booking through the stop it hangs on, not only through place_id', () => {
const files = [file({ id: 5, reservation_id: 90 })]
const bookings = [booking({ id: 90, assignment_id: 11 })]
expect(filesForPlace(files, 7, bookings, [11]).map(f => f.id)).toEqual([5])
// Without that stop in the day, the booking is not this place's business.
expect(filesForPlace(files, 7, bookings, [12])).toEqual([])
})
it('FE-UTIL-PLACEFILES-004: a file linked to the booking counts like one stored on it', () => {
const files = [file({ id: 6, linked_reservation_ids: [90] })]
expect(filesForPlace(files, 7, [booking({ id: 90, place_id: 7 })]).map(f => f.id)).toEqual([6])
})
it('FE-UTIL-PLACEFILES-005: lists a file once when it is both the place\'s and the booking\'s', () => {
const both = file({ id: 7, place_id: 7, reservation_id: 90 })
expect(filesForPlace([both], 7, [booking({ id: 90, place_id: 7 })]).map(f => f.id)).toEqual([7])
})
it('FE-UTIL-PLACEFILES-006: drops deleted files and answers empty without a place', () => {
const gone = file({ id: 8, place_id: 7, deleted_at: '2026-09-01' })
const bookingFileGone = file({ id: 9, reservation_id: 90, deleted_at: '2026-09-01' })
expect(filesForPlace([gone, bookingFileGone], 7, [booking({ id: 90, place_id: 7 })])).toEqual([])
expect(filesForPlace([file({ id: 10, place_id: 7 })], undefined, [])).toEqual([])
})
})