fix(planner): scheme-check the booking URL, and stop the serial routing calls

A booking's url is rendered as a plain href to every other member of the trip
and nothing ever checked its scheme, so a collaborator could store
javascript: there. The place website next to it has been going through
safeHttpUrl for exactly this reason, with a comment saying stored values
predate the server check - this is the surface that was left. Both ends now:
safeHttpUrl at the render site, and the scheme in the shared contract so
nothing new gets stored, additively so existing rows still load.

The per-leg routing rewrite turned one multi-waypoint request per run into
one serial request per leg. A twelve-stop day is eleven round trips to a
public OSRM in sequence before a single connector appears, and toggling km/mi
does it all again. Group consecutive legs that resolve to the same mode into
one request and fire the groups concurrently, keeping leg order on the join.

Transport reordering wrote to the store and persisted with a bare catch, so
a failed PUT left the sidebar showing an order the server never got; it
reconciles now. Plus the smaller ones: the OSM menu_url rendered unchecked,
expand/collapse-all persisting to a different storage than the day list reads,
the debounced address search with no ordering guard, the accommodation save
chain with no error path, and window.__trek_map holding a destroyed map.
This commit is contained in:
Maurice
2026-08-23 10:07:43 +02:00
committed by Maurice
parent 47a4276e84
commit 61d4fd4d64
18 changed files with 407 additions and 63 deletions
@@ -1696,6 +1696,15 @@ describe('MapViewGL', () => {
expect(glMap.remove).toHaveBeenCalled()
})
it('FE-COMP-MAPVIEWGL-069: unmounting lets go of the window debug handle', async () => {
const { unmount } = render(<MapViewGL places={[]} fitKey={1} />)
await act(async () => {})
expect((window as any).__trek_map).toBe(glMap)
unmount()
// Otherwise every style switch keeps its torn-down map reachable from window.
expect((window as any).__trek_map).toBeUndefined()
})
it('FE-COMP-MAPVIEWGL-059: pan and zoom report the visible bbox for the POI explore pill', async () => {
const onViewportChange = vi.fn()
render(<MapViewGL places={[]} fitKey={1} onViewportChange={onViewportChange} />)
+5
View File
@@ -930,6 +930,11 @@ export function MapViewGL({
}
try { map.remove() } catch { /* noop */ }
mapRef.current = null
// Drop the debug handle too, or a style switch keeps every torn-down map
// (canvas and sources included) alive for the rest of the page's life. The
// identity check leaves a freshly built map alone if this cleanup runs late.
// eslint-disable-next-line @typescript-eslint/no-explicit-any
if ((window as any).__trek_map === map) delete (window as any).__trek_map
setMapReady(false)
}
}, [glProvider, glStyle, mapboxToken, enableMapbox3d, mapboxQuality]) // rebuild on provider/style changes only
@@ -1,4 +1,4 @@
// FE-PLANNER-ADDRIN-001 to FE-PLANNER-ADDRIN-014
// FE-PLANNER-ADDRIN-001 to FE-PLANNER-ADDRIN-015
import { useState } from 'react';
import { delay, http, HttpResponse } from 'msw';
import userEvent from '@testing-library/user-event';
@@ -206,6 +206,28 @@ describe('AddressInput', () => {
expect(input).toHaveValue('Sacher');
});
it('FE-PLANNER-ADDRIN-015: a slow earlier search cannot overwrite the newer results', async () => {
// The debounce only cancels the timer; a request already on its way keeps
// going, and this one answers last.
server.use(http.post('/api/maps/search', async ({ request }) => {
const { query } = await request.json() as { query: string };
if (query === 'Sacher') { await delay(600); return HttpResponse.json({ places: [HOTEL] }); }
return HttpResponse.json({ places: [CAFE] });
}));
render(<Host />);
const input = screen.getByPlaceholderText('Street and number');
fireEvent.change(input, { target: { value: 'Sacher' } });
await settle(350);
fireEvent.change(input, { target: { value: 'Cafe Central' } });
expect(await screen.findByText('Cafe Central')).toBeInTheDocument();
// Now let the first request land.
await settle(500);
expect(screen.getByText('Cafe Central')).toBeInTheDocument();
expect(screen.queryByText('Hotel Sacher')).not.toBeInTheDocument();
});
it('FE-PLANNER-ADDRIN-014: a mousedown outside closes the list and a failing search empties it', async () => {
const user = userEvent.setup();
server.use(searchRoute([HOTEL]));
+11 -3
View File
@@ -22,6 +22,9 @@ export default function AddressInput({ value, onChange, placeholder, className }
const [loading, setLoading] = useState(false)
const wrapRef = useRef<HTMLDivElement>(null)
const debounceRef = useRef<ReturnType<typeof setTimeout> | null>(null)
// Clearing the timer does nothing to a request that is already out, and
// mapsApi.search takes no signal, so results are matched by ticket instead.
const reqIdRef = useRef(0)
useEffect(() => {
const handler = (e: MouseEvent) => {
@@ -38,23 +41,28 @@ export default function AddressInput({ value, onChange, placeholder, className }
const search = (text: string) => {
if (debounceRef.current) clearTimeout(debounceRef.current)
const trimmed = text.trim()
if (trimmed.length < 3) { setResults([]); setLoading(false); return }
// Same reason as in pick(): a request for the longer text may still land.
if (trimmed.length < 3) { reqIdRef.current++; setResults([]); setLoading(false); return }
debounceRef.current = setTimeout(async () => {
const myReq = ++reqIdRef.current
setLoading(true)
try {
const data = await mapsApi.search(trimmed, locale)
if (myReq !== reqIdRef.current) return
setResults(data.places || [])
setHighlight(-1)
} catch {
setResults([])
if (myReq === reqIdRef.current) setResults([])
} finally {
setLoading(false)
if (myReq === reqIdRef.current) setLoading(false)
}
}, 320)
}
const pick = (r: any) => {
if (debounceRef.current) clearTimeout(debounceRef.current)
// A response still on its way must not repopulate the list behind the pick.
reqIdRef.current++
onChange(r.address || r.name || '')
setOpen(false)
setResults([])
@@ -1,4 +1,4 @@
// FE-PLANNER-DAYDETAIL-001 to FE-PLANNER-DAYDETAIL-079
// FE-PLANNER-DAYDETAIL-001 to FE-PLANNER-DAYDETAIL-080
import React from 'react';
import { fireEvent, render, screen, waitFor, within, act } from '../../../tests/helpers/render';
import userEvent from '@testing-library/user-event';
@@ -342,6 +342,32 @@ describe('DayDetailPanel', () => {
expect(buttons.length).toBeGreaterThanOrEqual(2);
});
it('FE-PLANNER-DAYDETAIL-080: a failing accommodation edit keeps the picker open and says so', async () => {
const addToast = vi.fn();
window.__addToast = addToast;
server.use(
http.get('/api/trips/1/accommodations', () =>
HttpResponse.json({
accommodations: [{
id: 1, place_id: 5, place_name: 'Grand Hotel', place_address: 'Paris',
start_day_id: 1, end_day_id: 3, check_in: '14:00', check_out: null, confirmation: null,
}],
}),
),
http.put('/api/trips/1/accommodations/1', () => HttpResponse.json({ error: 'Stay overlaps' }, { status: 400 })),
);
render(<DayDetailPanel {...defaultProps} />);
await screen.findByText('Grand Hotel');
// The pencil beside the stay opens the picker in edit mode.
await userEvent.click(document.querySelector('.lucide-pencil')!.closest('button')!);
const picker = await waitFor(() => document.body.querySelector('[style*="z-index: 99999"]') as HTMLElement);
await userEvent.click(within(picker).getByText('Save'));
await waitFor(() => expect(addToast).toHaveBeenCalledWith('Stay overlaps', 'error', undefined));
// The picker stays put so the entered values are not lost.
expect(document.body.querySelector('[style*="z-index: 99999"]')).toBeInTheDocument();
});
it('FE-PLANNER-DAYDETAIL-022: accommodation edit/remove buttons hidden when canEditDays=false', async () => {
// Use regular user + restrict day_edit to admin only
const regularUser = buildUser({ id: 999, role: 'user' });
@@ -13,6 +13,7 @@ import { useTripStore } from '../../store/tripStore'
import CustomSelect from '../shared/CustomSelect'
import CustomTimePicker from '../shared/CustomTimePicker'
import { useSettingsStore } from '../../store/settingsStore'
import { useToast } from '../shared/Toast'
import { getLocaleForLanguage, useTranslation } from '../../i18n'
import type { Day, Place, Category, Reservation, AssignmentsMap } from '../../types'
import { isDayInAccommodationRange } from '../../utils/dayOrder'
@@ -492,6 +493,44 @@ function HotelPickerModal({ showHotelPicker, setShowHotelPicker, font, t, hotelD
days, locale, hotelForm, setHotelForm, categories, hotelCategoryFilter, setHotelCategoryFilter, places,
handleSelectPlace, accommodation, tripId, day, setAccommodations, setDayAccommodations, setAccommodation,
handleSaveAccommodation, onAccommodationChange }: any) {
const toast = useToast()
// Saving the picker. The edit branch stays here rather than in useDayDetail
// because it also refreshes the panel's own accommodation state; it has to say
// what went wrong when the write fails, otherwise the picker just sits there
// with the Save button doing nothing.
const saveHotelPicker = async () => {
if (showHotelPicker !== 'edit' || !accommodation) {
await handleSaveAccommodation()
return
}
try {
await accommodationsApi.update(tripId, accommodation.id, {
place_id: hotelForm.place_id,
start_day_id: hotelDayRange.start,
end_day_id: hotelDayRange.end,
check_in: hotelForm.check_in || null,
check_in_end: hotelForm.check_in_end || null,
check_out: hotelForm.check_out || null,
confirmation: hotelForm.confirmation || null,
})
setShowHotelPicker(false)
setHotelForm({ check_in: '', check_in_end: '', check_out: '', confirmation: '', place_id: null })
// Reload
const d = await accommodationsApi.list(tripId)
const all = d.accommodations || []
setAccommodations(all)
setDayAccommodations(all.filter(a =>
day ? isDayInAccommodationRange(day, a.start_day_id, a.end_day_id, days) : false
))
const acc = all.find(a => day ? isDayInAccommodationRange(day, a.start_day_id, a.end_day_id, days) : false)
setAccommodation(acc || null)
onAccommodationChange?.()
} catch (err: unknown) {
const message = (err as { response?: { data?: { error?: string } } })?.response?.data?.error
toast.error(message || t('common.unknownError'))
}
}
return (
<>
{/* Hotel Picker Popup — portal to body to escape transform stacking context */}
@@ -635,35 +674,7 @@ function HotelPickerModal({ showHotelPicker, setShowHotelPicker, font, t, hotelD
<button onClick={() => setShowHotelPicker(false)} style={{ padding: '7px 16px', borderRadius: 8, border: '1px solid var(--border-primary)', background: 'none', fontSize: 'calc(12px * var(--fs-scale-body, 1))', cursor: 'pointer', fontFamily: 'inherit', color: 'var(--text-muted)' }}>
{t('common.cancel')}
</button>
<button onClick={async () => {
if (showHotelPicker === 'edit' && accommodation) {
// Update existing
await accommodationsApi.update(tripId, accommodation.id, {
place_id: hotelForm.place_id,
start_day_id: hotelDayRange.start,
end_day_id: hotelDayRange.end,
check_in: hotelForm.check_in || null,
check_in_end: hotelForm.check_in_end || null,
check_out: hotelForm.check_out || null,
confirmation: hotelForm.confirmation || null,
})
setShowHotelPicker(false)
setHotelForm({ check_in: '', check_in_end: '', check_out: '', confirmation: '', place_id: null })
// Reload
accommodationsApi.list(tripId).then(d => {
const all = d.accommodations || []
setAccommodations(all)
setDayAccommodations(all.filter(a =>
day ? isDayInAccommodationRange(day, a.start_day_id, a.end_day_id, days) : false
))
const acc = all.find(a => day ? isDayInAccommodationRange(day, a.start_day_id, a.end_day_id, days) : false)
setAccommodation(acc || null)
})
onAccommodationChange?.()
} else {
await handleSaveAccommodation()
}
}} disabled={!hotelForm.place_id} style={{
<button onClick={() => { void saveHotelPicker() }} disabled={!hotelForm.place_id} style={{
padding: '7px 20px', borderRadius: 8, border: 'none', fontSize: 'calc(12px * var(--fs-scale-body, 1))', fontWeight: 600, cursor: 'pointer', fontFamily: 'inherit',
background: hotelForm.place_id ? 'var(--text-primary)' : 'var(--bg-tertiary)',
color: hotelForm.place_id ? 'var(--bg-card)' : 'var(--text-faint)',
@@ -2400,6 +2400,25 @@ describe('DayPlanSidebar', () => {
expect(stored!.day_plan_position).toBe(positions[1].day_plan_position)
})
it('FE-PLANNER-DAYPLAN-204: a rejected slot write puts the bookings back where the server has them', async () => {
const { reservationsApi } = await import('../../api/client')
vi.mocked(reservationsApi.updatePositions).mockRejectedValueOnce(new Error('offline'))
const place = buildPlace({ id: 1, name: 'Cafe', place_time: '08:00' })
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 train = buildReservation({ id: 302, type: 'train', title: 'Early train', reservation_time: '2025-06-01T06:00:00', day_id: 10 })
seedStore(useTripStore, { reservations: [train] })
render(<DayPlanSidebar {...makeDefaultProps({
days: [day], places: [place], assignments: { '10': [a] }, reservations: [train],
})} />)
await waitFor(() => expect(vi.mocked(reservationsApi.updatePositions)).toHaveBeenCalled())
// The optimistic slot is rolled back, so the next mount derives it again
// instead of showing an order only this tab knows about.
await waitFor(() => {
expect(useTripStore.getState().reservations.find(r => r.id === 302)!.day_plan_position).toBeNull()
})
})
it('FE-PLANNER-DAYPLAN-115: a multi-day cruise labels its start, middle and end days', () => {
const days = [
buildDay({ id: 10, date: '2025-06-01', title: 'Day 1' }),
@@ -2595,6 +2614,33 @@ describe('DayPlanSidebar', () => {
expect(screen.queryByText('2 km')).not.toBeInTheDocument()
})
it('FE-PLANNER-DAYPLAN-205: the legs of a day are fetched together, not one after the other', async () => {
const { calculateRouteWithLegs } = await import('../Map/RouteCalculator')
const release: (() => void)[] = []
vi.mocked(calculateRouteWithLegs as any).mockImplementation(() => new Promise(resolve => {
release.push(() => resolve({
coordinates: [], distance: 0, duration: 0,
legs: [{ distanceText: '2 km', durationText: '10 min', drivingText: '10 min', walkingText: '25 min' }],
}))
}))
const day = buildDay({ id: 10, date: '2025-06-01', title: 'Day 1' })
const places = [
buildPlace({ id: 1, name: 'A', lat: 48.85, lng: 2.35 }),
buildPlace({ id: 2, name: 'B', lat: 48.86, lng: 2.36 }),
buildPlace({ id: 3, name: 'C', lat: 48.87, lng: 2.37 }),
buildPlace({ id: 4, name: 'D', lat: 48.88, lng: 2.38 }),
]
const assignments = {
'10': places.map((place, i) => buildAssignment({ id: i + 1, day_id: 10, order_index: i, place })),
}
render(<DayPlanSidebar {...makeDefaultProps({ days: [day], places, assignments, selectedDayId: 10, routeShown: true })} />)
// All three legs are in flight while none of them has answered yet.
await waitFor(() => expect(release).toHaveLength(3))
release.forEach(fn => fn())
// And the connectors still land in one go once they come back.
await waitFor(() => expect(screen.getAllByText('2 km')).toHaveLength(3))
})
it('FE-PLANNER-DAYPLAN-123: adding a note to a collapsed day expands it first', async () => {
const user = userEvent.setup()
mockDayNotesState.openAddNote.mockImplementation((dayId: number, _getMerged: unknown, expand: (id: number) => void) => expand(dayId))
@@ -2664,6 +2710,31 @@ describe('DayPlanSidebar', () => {
await waitFor(() => expect(mockToast.error).toHaveBeenCalledWith('server said no'))
})
it('FE-PLANNER-DAYPLAN-206: a rejected slot write snaps the booking order back', async () => {
const { reservationsApi } = await import('../../api/client')
vi.mocked(reservationsApi.updatePositions).mockRejectedValueOnce(new Error('server said no'))
const day = buildDay({ id: 10, date: '2025-06-01', title: 'Day 1' })
const placeA = buildPlace({ id: 1, name: 'Place A' })
const placeB = buildPlace({ id: 2, name: 'Place B' })
const a1 = buildAssignment({ id: 11, day_id: 10, order_index: 0, place: placeA })
const a2 = buildAssignment({ id: 12, day_id: 10, order_index: 1, place: placeB })
const bus = buildReservation({ id: 430, type: 'bus', title: 'Airport bus', day_id: 10, day_plan_position: 1.5 })
seedStore(useTripStore, { reservations: [bus] })
render(<DayPlanSidebar {...makeDefaultProps({
days: [day], places: [placeA, placeB], assignments: { '10': [a1, a2] },
reservations: [bus], onReorder: vi.fn(async () => undefined),
})} />)
fireEvent.dragStart(dragRow(screen.getByText('Place B')), { dataTransfer: emptyDataTransfer })
fireEvent.drop(dragRow(screen.getByText('Place A')), { dataTransfer: { getData: vi.fn(() => '') } })
await waitFor(() => expect(mockToast.error).toHaveBeenCalledWith('server said no'))
// The optimistic slot goes with it — otherwise the day keeps an order the
// server never accepted.
const stored = useTripStore.getState().reservations.find(r => r.id === 430)!
expect(stored.day_plan_position).toBe(1.5)
expect(stored.day_positions).toBeUndefined()
})
it('FE-PLANNER-DAYPLAN-126: reordering around a multi-leg flight writes a position per leg', async () => {
const updateReservation = vi.fn(async () => ({} as Reservation))
stubTripActions({ updateReservation })
@@ -435,6 +435,10 @@ function useDayPlanSidebar(props: DayPlanSidebarProps) {
}))
// Mark as initialized immediately to prevent re-entry
for (const p of positions) initedTransportIds.current.add(p.id)
const before = positions.map(p => ({
id: p.id,
day_plan_position: useTripStore.getState().reservations.find(r => r.id === p.id)?.day_plan_position ?? null,
}))
// Update store so subscribers see the new positions
useTripStore.setState(state => ({
reservations: state.reservations.map(r => {
@@ -443,8 +447,20 @@ function useDayPlanSidebar(props: DayPlanSidebarProps) {
return { ...r, day_plan_position: p.day_plan_position }
})
}))
// Persist to server (fire and forget)
reservationsApi.updatePositions(tripId, positions).catch(() => {})
// Persist to server. No toast — this is a bootstrap write nobody asked for —
// but the store has to go back to what the server still holds, otherwise the
// day shows an order that only exists in this tab.
// The ids stay marked as initialised: the effect reruns on every reservations
// change, so clearing them here would retry the same failing write in a loop.
reservationsApi.updatePositions(tripId, positions).catch(() => {
useTripStore.setState(state => ({
reservations: state.reservations.map(r => {
const p = before.find(x => x.id === r.id)
if (!p) return r
return { ...r, day_plan_position: p.day_plan_position }
})
}))
})
}
const getMergedItems = (dayId: number): MergedItem[] =>
@@ -581,36 +597,67 @@ function useDayPlanSidebar(props: DayPlanSidebarProps) {
} catch { return undefined }
}
// Collect every leg of every day first, then fetch them a few at a time.
// Each result is stored under its own key, so nothing here depends on the
// order they come back in — which is what lets them overlap at all.
const tasks: (() => Promise<void>)[] = []
for (const dayId of routeDayIds) {
const { runs, startHotel, endHotel, firstWay, lastWay, wantTop, wantBottom } = planDay(dayId)
const dfMode = dayDefaultMode(dayId)
const dayLegs: Record<number, RouteSegment> = {}
legsByDay[dayId] = dayLegs
for (const run of runs) {
// One routing call per LEG, each with its own resolved mode, so a day can
// mix walking/driving/plugin legs. RouteCalculator's cache is keyed per
// profile+coords, so per-leg calls stay cheap (a single-mode day reuses
// the same entries) and each leg is tagged with the mode it was drawn in.
for (let i = 0; i < run.length - 1; i++) {
const seg = await legBetween({ lat: run[i].lat, lng: run[i].lng }, { lat: run[i + 1].lat, lng: run[i + 1].lng }, dayId, resolveLegMode(run[i], run[i + 1], dfMode))
if (seg) dayLegs[run[i].id] = seg
else if (controller.signal.aborted) return
const from = run[i]
const to = run[i + 1]
const mode = resolveLegMode(from, to, dfMode)
tasks.push(async () => {
const seg = await legBetween({ lat: from.lat, lng: from.lng }, { lat: to.lat, lng: to.lng }, dayId, mode)
if (seg) dayLegs[from.id] = seg
})
}
}
if (Object.keys(dayLegs).length) legsByDay[dayId] = dayLegs
const hotel: { top?: { seg: RouteSegment; name: string; targetId?: number }; bottom?: { seg: RouteSegment; name: string; targetId?: number } } = {}
hotelByDay[dayId] = hotel
if (wantTop) {
const seg = await legBetween({ lat: startHotel!.place_lat as number, lng: startHotel!.place_lng as number }, { lat: firstWay!.lat, lng: firstWay!.lng }, dayId, resolveLegMode({ isPlace: false }, firstWay!, dfMode))
if (seg) hotel.top = { seg, name: hotelName(startHotel!), targetId: firstWay!.isPlace ? firstWay!.id : undefined }
const mode = resolveLegMode({ isPlace: false }, firstWay!, dfMode)
tasks.push(async () => {
const seg = await legBetween({ lat: startHotel!.place_lat as number, lng: startHotel!.place_lng as number }, { lat: firstWay!.lat, lng: firstWay!.lng }, dayId, mode)
if (seg) hotel.top = { seg, name: hotelName(startHotel!), targetId: firstWay!.isPlace ? firstWay!.id : undefined }
})
}
if (wantBottom) {
const seg = await legBetween({ lat: lastWay!.lat, lng: lastWay!.lng }, { lat: endHotel!.place_lat as number, lng: endHotel!.place_lng as number }, dayId, resolveLegMode(lastWay!, { isPlace: false }, dfMode))
if (seg) hotel.bottom = { seg, name: hotelName(endHotel!), targetId: lastWay!.isPlace ? lastWay!.id : undefined }
const mode = resolveLegMode(lastWay!, { isPlace: false }, dfMode)
tasks.push(async () => {
const seg = await legBetween({ lat: lastWay!.lat, lng: lastWay!.lng }, { lat: endHotel!.place_lat as number, lng: endHotel!.place_lng as number }, dayId, mode)
if (seg) hotel.bottom = { seg, name: hotelName(endHotel!), targetId: lastWay!.isPlace ? lastWay!.id : undefined }
})
}
if (controller.signal.aborted) return
if (hotel.top || hotel.bottom) hotelByDay[dayId] = hotel
}
if (!controller.signal.aborted) { setRouteLegs(legsByDay); setHotelLegs(hotelByDay) }
// Small pool on purpose: a week of days is dozens of legs and the routing
// host is often a shared OSRM.
let nextTask = 0
const runTasks = async () => {
while (nextTask < tasks.length && !controller.signal.aborted) await tasks[nextTask++]()
}
await Promise.all(Array.from({ length: Math.min(6, tasks.length) }, runTasks))
if (controller.signal.aborted) return
// Days that ended up without a single leg were never in the map before.
for (const dayId of Object.keys(legsByDay).map(Number)) {
if (!Object.keys(legsByDay[dayId]).length) delete legsByDay[dayId]
}
for (const dayId of Object.keys(hotelByDay).map(Number)) {
if (!hotelByDay[dayId].top && !hotelByDay[dayId].bottom) delete hotelByDay[dayId]
}
setRouteLegs(legsByDay)
setHotelLegs(hotelByDay)
})()
// routeDayIds is memoized from the same inputs as routeDayKey below, so keying the
// effect on the string is equivalent while staying stable across unrelated renders.
@@ -642,6 +689,9 @@ function useDayPlanSidebar(props: DayPlanSidebarProps) {
const applyMergedOrder = async (dayId: number, newOrder: { type: string; data: any }[]) => {
// Capture previous place order for undo
const prevAssignmentIds = getDayAssignments(dayId).map(a => a.id)
// …and the reservations as they stand, so a failed write can put the visible
// order back instead of leaving a phantom one behind the error toast.
const prevReservations = useTripStore.getState().reservations
// Places get sequential integer positions (0, 1, 2, ...)
// Non-place items between place N-1 and place N get fractional positions
@@ -732,7 +782,11 @@ function useDayPlanSidebar(props: DayPlanSidebarProps) {
await tripActions.reorderAssignments(tripId, capturedDayId, capturedPrevIds)
})
}
} catch (err: unknown) { toast.error(err instanceof Error ? err.message : t('common.unknownError')) }
} catch (err: unknown) {
useTripStore.setState({ reservations: prevReservations })
setTransportPosVersion(v => v + 1)
toast.error(err instanceof Error ? err.message : t('common.unknownError'))
}
}
const handleMergedDrop = async (dayId, fromType, fromId, toType, toId, insertAfter = false, toLegIndex = null) => {
@@ -67,6 +67,7 @@ const routableReservation = () => buildReservation({
beforeEach(() => {
vi.clearAllMocks()
sessionStorage.clear()
localStorage.clear()
})
describe('DayPlanSidebarToolbar', () => {
@@ -142,7 +143,10 @@ describe('DayPlanSidebarToolbar', () => {
expect(btn).toHaveAttribute('aria-pressed', 'false')
await user.click(btn)
expect(setExpandedDays).toHaveBeenCalledWith(new Set([10, 11]))
expect(JSON.parse(sessionStorage.getItem('day-expanded-1')!)).toEqual([10, 11])
// The sidebar restores the set from localStorage on mount, so that is the
// store the toolbar has to write to.
expect(JSON.parse(localStorage.getItem('day-expanded-1')!)).toEqual([10, 11])
expect(sessionStorage.getItem('day-expanded-1')).toBeNull()
})
it('FE-PLANNER-DPTOOLBAR-012: with every day expanded the button collapses them all', async () => {
@@ -154,7 +158,7 @@ describe('DayPlanSidebarToolbar', () => {
expect(btn).toHaveAttribute('aria-pressed', 'true')
await user.click(btn)
expect(setExpandedDays).toHaveBeenCalledWith(new Set())
expect(JSON.parse(sessionStorage.getItem('day-expanded-1')!)).toEqual([])
expect(JSON.parse(localStorage.getItem('day-expanded-1')!)).toEqual([])
})
it('FE-PLANNER-DPTOOLBAR-013: a trip with no days is never treated as fully expanded', () => {
@@ -98,7 +98,9 @@ export function DayPlanSidebarToolbar({
onClick={() => {
const next = allExpanded ? new Set<number>() : new Set(days.map(d => d.id))
setExpandedDays(next)
try { sessionStorage.setItem(`day-expanded-${tripId}`, JSON.stringify([...next])) } catch {}
// Same store the sidebar reads on mount — a sessionStorage write
// here left the persisted set behind after a reload.
try { localStorage.setItem(`day-expanded-${tripId}`, JSON.stringify([...next])) } catch {}
}}
aria-label={label}
aria-pressed={allExpanded}
@@ -208,6 +208,20 @@ describe('PlaceDetailsColumn', () => {
)
})
it('FE-PDC-038: a fact url that is not http(s) stays a chip, never a link', async () => {
// fact.url comes straight from an OSM tag anyone can edit, so a
// javascript: value must not end up in an href.
placeEnrichment.mockResolvedValue({
photos: [],
description: null,
facts: [{ kind: 'menu', value: 'Menu', url: 'javascript:alert(1)' }],
})
renderColumn()
expect(await screen.findByText('Menu')).toBeInTheDocument()
expect(screen.queryByRole('link', { name: /Menu/ })).not.toBeInTheDocument()
})
it('FE-PDC-017: renders facts even when there is neither a picture nor a description', async () => {
placeEnrichment.mockResolvedValue({
photos: [],
@@ -26,6 +26,7 @@ import type { MapsPlaceEnrichmentResult, PlaceFact, PlaceHours, PlacePhotoCandid
import { mapsApi } from '../../api/client'
import { resolveOpenNow, resolvePlaceTimeZone, placeWeekdayIndex } from './placeOpenState'
import { convertHoursLine, isUnknownHoursLine, splitHoursLine } from './placeHoursFormat'
import { safeHttpUrl } from '../../utils/safeUrl'
import type { TranslationFn } from '../../types'
/** The place the column is describing. Null while nothing is selected. */
@@ -616,11 +617,14 @@ function FactList({ facts, t }: { facts: PlaceFact[]; t: TranslationFn }): React
</>
)
const shared = 'inline-flex items-center gap-1.5 max-w-full rounded-full border border-edge bg-surface px-2 py-1 text-caption text-content-secondary'
// A fact url is a community-edited OSM tag (website:menu and friends),
// so anything but http(s) renders as the plain chip instead of a link.
const href = safeHttpUrl(fact.url)
return fact.url ? (
return href ? (
<a
key={fact.kind}
href={fact.url}
href={href}
target="_blank"
rel="noopener noreferrer"
title={label}
@@ -1,4 +1,4 @@
// FE-COMP-RES-001 to FE-COMP-RES-040, FE-PLANNER-RESP-016 to FE-PLANNER-RESP-078
// FE-COMP-RES-001 to FE-COMP-RES-040, FE-PLANNER-RESP-016 to FE-PLANNER-RESP-080
import { render, screen, fireEvent, waitFor, act } from '../../../tests/helpers/render';
import userEvent from '@testing-library/user-event';
import { http, HttpResponse } from 'msw';
@@ -900,4 +900,21 @@ describe('ReservationsPanel', () => {
const text = document.body.textContent || '';
expect(text.indexOf('Dated train')).toBeLessThan(text.indexOf('Undated taxi'));
});
it('FE-PLANNER-RESP-079: an http booking link is rendered as an anchor', () => {
const res = buildReservation({ id: 1, title: 'Hotel', status: 'confirmed', url: 'https://hotel.example/booking' } as any);
render(<ReservationsPanel {...defaultProps} reservations={[res]} />);
const link = screen.getByText('https://hotel.example/booking');
expect(link.tagName).toBe('A');
expect(link).toHaveAttribute('href', 'https://hotel.example/booking');
});
it('FE-PLANNER-RESP-080: a javascript: booking link stays plain text, never an href', () => {
const res = buildReservation({ id: 1, title: 'Hotel', status: 'confirmed', url: 'javascript:alert(1)' } as any);
render(<ReservationsPanel {...defaultProps} reservations={[res]} />);
// The stored value stays readable, but nothing is clickable.
const shown = screen.getByText('javascript:alert(1)');
expect(shown.tagName).toBe('SPAN');
expect(document.querySelector('a[href^="javascript:"]')).toBeNull();
});
});
@@ -12,6 +12,7 @@ import {
TramFront, Footprints, StickyNote, ParkingSquare,
} from 'lucide-react'
import { openFile } from '../../utils/fileDownload'
import { safeHttpUrl } from '../../utils/safeUrl'
import { TransitTitle, TransitLegChips, TransitMetaBadges, fmtTransitDuration } from './transitDisplay'
import Markdown from 'react-markdown'
import remarkGfm from 'remark-gfm'
@@ -116,6 +117,9 @@ function ReservationCard({ r, tripId, onEdit, onDelete, files = [], onNavigateTo
})()
const attachedFiles = files.filter(f => f.reservation_id === r.id || (f.linked_reservation_ids || []).includes(r.id))
const linked = r.assignment_id ? assignmentLookup[r.assignment_id] : null
// A booking link is free-form text on rows written before the contract checked it,
// so only an http(s) value becomes a real anchor — the rest stays readable text.
const bookingUrl = safeHttpUrl(r.url)
const [showDeleteConfirm, setShowDeleteConfirm] = useState(false)
const handleDelete = async () => {
@@ -432,8 +436,12 @@ function ReservationCard({ r, tripId, onEdit, onDelete, files = [], onNavigateTo
<div className={fieldLabelClass}>{t('reservations.urlLabel')}</div>
<div className={fieldValueClass} style={{ display: 'flex', alignItems: 'center', gap: 6, fontWeight: 400 }}>
<ExternalLink size={13} className="text-content-faint" style={{ flexShrink: 0 }} />
<a href={r.url} target="_blank" rel="noopener noreferrer" className="text-accent hover:underline"
style={{ overflow: 'hidden', textOverflow: 'ellipsis', whiteSpace: 'nowrap' }}>{r.url}</a>
{bookingUrl ? (
<a href={bookingUrl} target="_blank" rel="noopener noreferrer" className="text-accent hover:underline"
style={{ overflow: 'hidden', textOverflow: 'ellipsis', whiteSpace: 'nowrap' }}>{bookingUrl}</a>
) : (
<span style={{ overflow: 'hidden', textOverflow: 'ellipsis', whiteSpace: 'nowrap' }}>{r.url}</span>
)}
</div>
</div>
)}
+14 -5
View File
@@ -205,19 +205,28 @@ export function useRouteCalculation(tripStore: TripStoreState, selectedDayId: nu
polyline.push(c)
}
}
for (let i = 0; i < run.length - 1; i++) {
// Neighbouring legs that resolve to the SAME mode travel as one
// multi-waypoint request — the router answers with one leg per pair, so
// the per-connector segments stay exactly as they were, and a day without
// per-leg overrides is back to a single call per run.
let i = 0
while (i < run.length - 1) {
const mode = resolveLegMode(run[i], run[i + 1], dayDefaultMode)
const straight = (): [number, number][] => [[run[i].lat, run[i].lng], [run[i + 1].lat, run[i + 1].lng]]
let end = i + 1
while (end < run.length - 1 && resolveLegMode(run[end], run[end + 1], dayDefaultMode) === mode) end++
const chunk = run.slice(i, end + 1)
const straight = (): [number, number][] => chunk.map(p => [p.lat, p.lng] as [number, number])
try {
const r = await calculateRouteWithLegs([{ lat: run[i].lat, lng: run[i].lng }, { lat: run[i + 1].lat, lng: run[i + 1].lng }], { signal: controller.signal, profile: mode, tripId, dayId })
const r = await calculateRouteWithLegs(chunk.map(p => ({ lat: p.lat, lng: p.lng })), { signal: controller.signal, profile: mode, tripId, dayId })
pushCoords(r.coordinates.length >= 2 ? r.coordinates : straight())
if (r.legs[0]) allLegs.push({ ...r.legs[0], mode })
for (const leg of r.legs) allLegs.push({ ...leg, mode })
if (r.vias) allVias.push(...r.vias)
} catch (err) {
if (err instanceof Error && err.name === 'AbortError') throw err
// Routing failed for this leg — fall back to a straight line, no times.
// Routing failed for these legs — fall back to straight lines, no times.
pushCoords(straight())
}
i = end
}
if (polyline.length >= 2) polylines.push(polyline)
}
@@ -117,6 +117,48 @@ describe('useRouteCalculation', () => {
expect(result.current.routeSegments).toEqual(MOCK_SEGMENTS.map(s => ({ ...s, mode: 'driving' })));
});
it('FE-HOOK-ROUTE-023: consecutive legs of one mode go out as a single multi-waypoint request', async () => {
// Three places routed in the day default and a fourth reached on foot: two
// requests, not three, and the connectors stay one per pair in order.
(calculateRouteWithLegs as ReturnType<typeof vi.fn>).mockImplementation(
(waypoints: { lat: number; lng: number }[]) => Promise.resolve({
coordinates: [] as [number, number][],
distance: 0,
duration: 0,
legs: waypoints.slice(0, -1).map((w, i) => ({
...MOCK_SEGMENTS[0],
from: [w.lat, w.lng] as [number, number],
to: [waypoints[i + 1].lat, waypoints[i + 1].lng] as [number, number],
})),
}),
);
const pts = [
{ lat: 48.86, lng: 2.35 }, { lat: 48.87, lng: 2.36 },
{ lat: 48.88, lng: 2.37 }, { lat: 48.89, lng: 2.38 },
];
const assignments = pts.map((pt, i) => ({
...buildAssignment({ day_id: 5, order_index: i, place: buildPlace(pt) }),
// The third place walks to the fourth; the first two use the day default.
leg_transport_mode: i === 2 ? 'walking' : null,
}));
const store = buildMockStore({ '5': assignments as never });
const { result } = renderHook(() => useRouteCalculation(store as TripStoreState, 5));
await act(async () => {});
const calls = (calculateRouteWithLegs as ReturnType<typeof vi.fn>).mock.calls;
expect(calls).toHaveLength(2);
expect(calls[0][0]).toEqual(pts.slice(0, 3));
expect(calls[0][1].profile).toBe('driving');
expect(calls[1][0]).toEqual(pts.slice(2));
expect(calls[1][1].profile).toBe('walking');
// One segment per pair, in the order they are walked.
expect(result.current.routeSegments.map(s => s.mode)).toEqual(['driving', 'driving', 'walking']);
expect(result.current.routeSegments.map(s => s.from)).toEqual([
[pts[0].lat, pts[0].lng], [pts[1].lat, pts[1].lng], [pts[2].lat, pts[2].lng],
]);
});
it('FE-HOOK-ROUTE-006: assignments are sorted by order_index before extracting waypoints', async () => {
const p1 = buildPlace({ lat: 10, lng: 10 });
const p2 = buildPlace({ lat: 20, lng: 20 });
@@ -1,5 +1,6 @@
import {
reservationCreateRequestSchema,
reservationUpdateRequestSchema,
reservationPositionsRequestSchema,
accommodationCreateBodySchema,
accommodationCreateRequestSchema,
@@ -20,6 +21,23 @@ describe('reservationCreateRequestSchema', () => {
).toBe(true);
expect(reservationCreateRequestSchema.safeParse({ location: 'x' }).success).toBe(false);
});
it('rejects a booking url that executes instead of navigating, on create and update', () => {
expect(reservationCreateRequestSchema.safeParse({ title: 'Hotel', url: 'javascript:alert(1)' }).success).toBe(false);
expect(reservationCreateRequestSchema.safeParse({ title: 'Hotel', url: 'JavaScript:alert(1)' }).success).toBe(false);
expect(reservationCreateRequestSchema.safeParse({ title: 'Hotel', url: ' \tjava\nscript:alert(1)' }).success).toBe(false);
expect(reservationCreateRequestSchema.safeParse({ title: 'Hotel', url: 'data:text/html,<script>x</script>' }).success).toBe(false);
expect(reservationUpdateRequestSchema.safeParse({ url: 'vbscript:msgbox' }).success).toBe(false);
});
it('keeps every link people actually paste valid (no stored row becomes unsavable)', () => {
for (const url of ['https://hotel.example/booking', 'http://hotel.example', 'www.hotel.com', 'hotel.example/x?a=b', '']) {
expect(reservationCreateRequestSchema.safeParse({ title: 'Hotel', url }).success).toBe(true);
expect(reservationUpdateRequestSchema.safeParse({ url }).success).toBe(true);
}
expect(reservationUpdateRequestSchema.safeParse({ url: null }).success).toBe(true);
expect(reservationUpdateRequestSchema.safeParse({ location: 'x' }).success).toBe(true);
});
});
describe('reservationPositionsRequestSchema', () => {
+22 -2
View File
@@ -202,11 +202,31 @@ export const accommodationSchema = z.object({
});
export type Accommodation = z.infer<typeof accommodationSchema>;
/**
* A booking link, as it is rendered into an href by the reservations panel. It
* deliberately stays free-form — people paste bare hosts ("www.hotel.com") and
* provider deep links, and rows written before this check exist — so the contract
* only rejects the schemes that execute in this origin instead of opening a page.
* Same class of bug as placeWebsiteSchema, one step narrower so nobody's stored
* link becomes unsavable.
*/
export const reservationUrlSchema = z.string().refine(
// Browsers strip control characters and whitespace before they resolve the
// scheme, so a tab spliced into 'javascript:' still runs.
v => !/^(javascript|data|vbscript):/i.test(v.replace(/[\u0000-\u0020]/g, '')),
{ message: 'must not be a javascript:, data: or vbscript: URL' },
);
/** Reservation create: title is required; the many optional fields stay open. */
export const reservationCreateRequestSchema = open.and(z.object({ title: z.string().min(1) }));
export const reservationCreateRequestSchema = open.and(z.object({
title: z.string().min(1),
url: reservationUrlSchema.nullable().optional(),
}));
export type ReservationCreateRequest = z.infer<typeof reservationCreateRequestSchema>;
export const reservationUpdateRequestSchema = open;
export const reservationUpdateRequestSchema = open.and(z.object({
url: reservationUrlSchema.nullable().optional(),
}));
export type ReservationUpdateRequest = z.infer<typeof reservationUpdateRequestSchema>;
/** Assign trip members/guests to a reservation (mirrors budget's PUT :id/members). */