diff --git a/client/src/components/Planner/PlaceInspector.tsx b/client/src/components/Planner/PlaceInspector.tsx index c895f25e1..504f4361a 100644 --- a/client/src/components/Planner/PlaceInspector.tsx +++ b/client/src/components/Planner/PlaceInspector.tsx @@ -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 (
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)) { diff --git a/client/src/components/Planner/assignmentOptions.ts b/client/src/components/Planner/assignmentOptions.ts new file mode 100644 index 000000000..1a53dfd16 --- /dev/null +++ b/client/src/components/Planner/assignmentOptions.ts @@ -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 | 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 +} diff --git a/client/src/mobile/screens/trip/sheets/MPlaceSheet.tsx b/client/src/mobile/screens/trip/sheets/MPlaceSheet.tsx index 968c214a1..b3ca996a5 100644 --- a/client/src/mobile/screens/trip/sheets/MPlaceSheet.tsx +++ b/client/src/mobile/screens/trip/sheets/MPlaceSheet.tsx @@ -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) => { diff --git a/client/src/mobile/screens/trip/sheets/MReservationSheet.tsx b/client/src/mobile/screens/trip/sheets/MReservationSheet.tsx index 81b037b88..ca219a584 100644 --- a/client/src/mobile/screens/trip/sheets/MReservationSheet.tsx +++ b/client/src/mobile/screens/trip/sheets/MReservationSheet.tsx @@ -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([]) + 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>(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('') 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
)} + {/* LINK TO A STOP IN THE PLAN (#2216) */} + {!isHotel && assignmentOptions.length > 0 && ( + <> + {t('reservations.linkAssignment')} + { + 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 && ( + <> + {t('files.title')} +
+ {attachedFiles.map(f => ( + + ))} +
+ + )} {canUploadFiles && ( + !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)) + }), + ] +} diff --git a/client/tests/unit/mobile/trip/MReservationSheet.test.tsx b/client/tests/unit/mobile/trip/MReservationSheet.test.tsx index f40077a2a..460d1070b 100644 --- a/client/tests/unit/mobile/trip/MReservationSheet.test.tsx +++ b/client/tests/unit/mobile/trip/MReservationSheet.test.tsx @@ -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() diff --git a/client/tests/unit/utils/placeFiles.test.ts b/client/tests/unit/utils/placeFiles.test.ts new file mode 100644 index 000000000..e8f3547a3 --- /dev/null +++ b/client/tests/unit/utils/placeFiles.test.ts @@ -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 => ({ + 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 => ({ + 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([]) + }) +})