fix(bookings): show where an imported stay lands on the map before it is saved

An imported hotel was looked up once, by its name and its address together.
A booking site's romanized address (Agoda for a stay in Japan) often matches
nothing and sank the hotel's name with it, so the place was created without
coordinates and silently never showed on the map.

The review now finds the stay right away (name and address, then the name,
then the address) and says under the address whether it is on the map and
where, or that it is not yet, with a way to search again. Picking an address
suggestion sets the spot by hand. The save uses that spot, falls back the same
way when there is none, and a stay saved without a spot says so in a toast.
Desktop dialog and phone sheet share the lookup through useReservationForm.
This commit is contained in:
Maurice
2026-10-25 18:14:29 +01:00
parent a0b7b4d196
commit b80fdf27f2
41 changed files with 668 additions and 26 deletions
@@ -5,19 +5,22 @@ import { useLocationBias } from '../../hooks/useLocationBias'
import { usePlaceLanguage } from '../../hooks/usePlaceLanguage'
import { useSuggestionDropdown } from './useSuggestionDropdown'
import { PlaceSuggestion, SuggestionList, SuggestionRow } from './SuggestionDropdown'
import type { MapMatch } from '../../repo/mapMatchRepo'
interface Props {
value: string
onChange: (value: string) => void
placeholder?: string
className?: string
/** A picked suggestion's spot on the map, for a form that saves where the address is (an imported stay). */
onPick?: (match: MapMatch) => void
}
// Free-text address input with location autocomplete, backed by the same maps
// search as LocationSelect. Unlike LocationSelect the typed text is
// authoritative: every keystroke reaches the parent so a hand-written address
// is never lost, and picking a suggestion just replaces the text (#1496).
export default function AddressInput({ value, onChange, placeholder, className }: Props) {
export default function AddressInput({ value, onChange, placeholder, className, onPick }: Props) {
const { locale } = useTranslation()
const placeLang = usePlaceLanguage()
// Ohne Reisekontext ist der Hinweis leer, und die Suche laeuft wie bisher.
@@ -60,6 +63,9 @@ export default function AddressInput({ value, onChange, placeholder, className }
// A response still on its way must not repopulate the list behind the pick.
reqIdRef.current++
onChange(r.address || r.name || '')
if (onPick && Number.isFinite(r.lat) && Number.isFinite(r.lng)) {
onPick({ lat: r.lat, lng: r.lng, label: [r.name, r.address].filter(Boolean).join(', ') })
}
setOpen(false)
setResults([])
setLoading(false)
@@ -29,6 +29,7 @@ import { useReservationForm } from './useReservationForm'
import { expenseRequestAfterSave, pendingImportExpense, travelerIdsOf, travelersChanged, filesNeedConnection, uploadBookingFiles } from './bookingFormModel'
import { useBookingExpenseIntent } from './useBookingExpenseIntent'
import { useBookingFileAttach } from './useBookingFileAttach'
import StayPinStatus from './StayPinStatus'
const EMPTY_FORM = {
title: '', type: 'other', status: 'pending',
@@ -81,7 +82,7 @@ export function ReservationModal({ isOpen, onClose, onSave, reservation, days, p
const {
form, setForm, set, isSaving, setIsSaving, pendingFiles, setPendingFiles, travelerIds, setTravelerIds, toggleTraveler,
isEndBeforeStart, endTimeBeforeStartTime, dateBounds: tripDateRange, startDate, startTime, setStartDate, setStartTime, takeStopDay,
pickPlace, pickHotelPlace, pickHotelStart, pickHotelEnd, saveData: buildSaveData,
pickPlace, pickHotelPlace, pickHotelStart, pickHotelEnd, stayPin, saveData: buildSaveData,
} = useReservationForm(EMPTY_FORM, days, places)
const attach = useBookingFileAttach({ reservation, files, onFileUpload, setPendingFiles, toast, t })
@@ -102,17 +103,22 @@ export function ReservationModal({ isOpen, onClose, onSave, reservation, days, p
}
setTravelerIds(travelerIdsOf(reservation))
stayPin.reset()
if (reservation) {
setForm({ ...reservationFieldsFrom(reservation, accommodations, places, true), assignment_id: reservation.assignment_id || '' })
} else if (prefill) {
// Review-before-save: populate from a parsed import item, stay in create mode.
const hotelPlaceId = matchPlaceId(prefill._venue?.name || prefill.title)
setForm({
...reservationFieldsFromPrefill(prefill, days, true),
assignment_id: defaultAssignmentId ?? '',
accommodation_id: '',
place_id: '',
hotel_place_id: matchPlaceId(prefill._venue?.name || prefill.title),
hotel_place_id: hotelPlaceId,
})
// A stay the trip does not hold yet: find it on the map now, so the review
// says whether it will show there before it is saved.
if (prefill.type === 'hotel' && !hotelPlaceId) stayPin.lookUp(prefill._venue?.name || prefill.title, prefill._venue?.address)
// Seed the booking's Files with the document this item was parsed from.
setPendingFiles(prefill._sourceFiles ?? [])
} else {
@@ -362,8 +368,12 @@ export function ReservationModal({ isOpen, onClose, onSave, reservation, days, p
</div>
<EditorField label={t('reservations.locationAddress')}>
<AddressInput value={form.hotel_address} onChange={v => set('hotel_address', v)}
onPick={!form.hotel_place_id ? stayPin.pick : undefined}
placeholder={t('reservations.locationPlaceholder')} className={INPUT} />
</EditorField>
{!form.hotel_place_id && (
<StayPinStatus pin={stayPin.pin} onRetry={() => stayPin.lookUp(form.title, form.hotel_address)} />
)}
</>
)}
@@ -0,0 +1,43 @@
import { fireEvent, render, screen } from '@testing-library/react';
import { createElement } from 'react';
import { describe, expect, it, vi } from 'vitest';
import { TranslationProvider } from '../../i18n/TranslationContext';
import StayPinStatus from './StayPinStatus';
import type { StayPin } from './useStayPin';
function show(pin: StayPin, onRetry = vi.fn()) {
render(createElement(TranslationProvider, null, createElement(StayPinStatus, { pin, onRetry })));
return onRetry;
}
describe('StayPinStatus', () => {
it('FE-STAY-PIN-STATUS-001: shows nothing before a lookup', () => {
const { container } = render(
createElement(
TranslationProvider,
null,
createElement(StayPinStatus, { pin: { status: 'idle' }, onRetry: vi.fn() })
)
);
expect(container.textContent).toBe('');
});
it('FE-STAY-PIN-STATUS-002: says it is looking, then where it found the stay', () => {
show({ status: 'searching' });
expect(screen.getByRole('status')).toHaveTextContent('Looking for it on the map');
});
it('FE-STAY-PIN-STATUS-003: a found stay names the match, so a wrong one is plain', () => {
show({ status: 'found', match: { lat: 1, lng: 2, label: 'Hotel Gracery Shinjuku, Tokyo' } });
expect(screen.getByRole('status')).toHaveTextContent('On the map');
expect(screen.getByText('Hotel Gracery Shinjuku, Tokyo')).toBeInTheDocument();
});
it('FE-STAY-PIN-STATUS-004: a missing stay says so and offers to search again', () => {
const onRetry = show({ status: 'missing' });
expect(screen.getByText('Not on the map yet')).toBeInTheDocument();
fireEvent.click(screen.getByRole('button', { name: /search again/i }));
expect(onRetry).toHaveBeenCalledTimes(1);
});
});
@@ -0,0 +1,60 @@
import { MapPin, MapPinOff, RotateCw } from 'lucide-react';
import { useTranslation } from '../../i18n';
import type { StayPin } from './useStayPin';
interface StayPinStatusProps {
pin: StayPin;
/** Asks the map again with what the form now holds. */
onRetry: () => void;
}
/**
* Whether an imported stay will show on the map, under its address in the
* review: being looked up, found (and where, so a wrong match is plain), or not
* found, with a way to search again before it is saved without a pin. Shared by
* the desktop booking dialog and the phone's booking sheet.
*/
export default function StayPinStatus({ pin, onRetry }: StayPinStatusProps) {
const { t } = useTranslation();
if (pin.status === 'idle') return null;
if (pin.status === 'searching') {
return (
<div role="status" className="flex items-center gap-2 px-1 text-caption text-content-muted">
<span className="h-3.5 w-3.5 flex-none animate-spin rounded-full border-2 border-edge border-t-content-muted" />
{t('reservations.import.pinSearching')}
</div>
);
}
if (pin.status === 'found') {
return (
<div role="status" className="flex min-w-0 items-center gap-2 rounded-lg bg-success-soft px-3 py-2 text-caption">
<MapPin size={14} strokeWidth={2.2} className="flex-none text-success" />
<span className="flex-none font-semibold text-success">{t('reservations.import.pinFound')}</span>
<span className="min-w-0 truncate text-content-secondary" title={pin.match.label}>
{pin.match.label}
</span>
</div>
);
}
return (
<div role="status" className="flex items-start gap-3 rounded-xl bg-warning-soft px-3.5 py-3">
<MapPinOff size={16} strokeWidth={2.2} className="mt-0.5 flex-none text-warning" />
<div className="min-w-0 flex-1">
<p className="m-0 text-body font-semibold text-content">{t('reservations.import.pinMissingTitle')}</p>
<p className="m-0 mt-0.5 text-caption text-content-muted">{t('reservations.import.pinMissingBody')}</p>
<button
type="button"
onClick={onRetry}
className="mt-2 inline-flex items-center gap-1.5 rounded-lg border border-edge bg-surface-card px-2.5 py-1 text-caption font-medium text-content hover:bg-surface-hover"
>
<RotateCw size={12} strokeWidth={2.2} />
{t('reservations.import.pinRetry')}
</button>
</div>
</div>
);
}
@@ -287,6 +287,8 @@ export interface ReservationSaveOptions {
isEdit: boolean;
/** The desktop also saves the end of the check-in window. */
withCheckInEnd: boolean;
/** Where the review found an imported stay on the map, so the save creates its place right there. */
venuePin?: { lat: number; lng: number } | null;
}
export type ReservationSaveData = Record<string, unknown> & { title: string };
@@ -344,7 +346,11 @@ export function reservationSaveData(
// import): the save handler geocodes it and creates the place.
venue:
!form.hotel_place_id && (form.hotel_address || form.title)
? { name: form.title, address: form.hotel_address || null }
? {
name: form.title,
address: form.hotel_address || null,
...(opts.venuePin ? { lat: opts.venuePin.lat, lng: opts.venuePin.lng } : {}),
}
: null,
// The typed address, so the save handler can write it through to a linked
// place; an edited address used to be silently dropped (#1496).
@@ -19,6 +19,7 @@ import {
withStartDate,
withStartTime,
} from './reservationFormModel';
import { useStayPin } from './useStayPin';
/**
* The booking form behind the desktop ReservationModal and the phone's booking
@@ -39,6 +40,8 @@ export function useReservationForm<F extends ReservationFields & CheckInEndField
const set = (field: keyof F, value: string | number) => setForm((prev) => ({ ...prev, [field]: value }));
const toggleTraveler = (id: number) => setTravelerIds((prev) => toggledTraveler(prev, id));
// Where an imported stay will land on the map, looked up while it is reviewed.
const stayPin = useStayPin();
// Restrict non-hotel booking dates to the trip's span (#1662). Hotels already
// constrain to trip days via their day dropdowns.
@@ -71,6 +74,7 @@ export function useReservationForm<F extends ReservationFields & CheckInEndField
pickHotelPlace: (value: string | number) => setForm((prev) => withHotelPlace(prev, value, places)),
pickHotelStart: (value: string | number) => setForm((prev) => withHotelStart(prev, value, days)),
pickHotelEnd: (value: string | number) => setForm((prev) => withHotelEnd(prev, value, days)),
saveData: (opts: ReservationSaveOptions) => reservationSaveData(form, opts),
stayPin,
saveData: (opts: ReservationSaveOptions) => reservationSaveData(form, { ...opts, venuePin: stayPin.match }),
};
}
@@ -0,0 +1,74 @@
import { act, renderHook, waitFor } from '@testing-library/react';
import { beforeEach, describe, expect, it, vi } from 'vitest';
import { findOnMap } from '../../repo/mapMatchRepo';
import { isEffectivelyOffline } from '../../sync/networkMode';
import { useStayPin } from './useStayPin';
vi.mock('../../hooks/usePlaceLanguage', () => ({ usePlaceLanguage: () => 'en' }));
vi.mock('../../sync/networkMode', () => ({ isEffectivelyOffline: vi.fn(() => false) }));
vi.mock('../../repo/mapMatchRepo', async (importOriginal) => ({
...(await importOriginal<typeof import('../../repo/mapMatchRepo')>()),
findOnMap: vi.fn(),
}));
const SPOT = { lat: 35.69, lng: 139.7, label: 'Hotel Gracery Shinjuku' };
beforeEach(() => {
vi.clearAllMocks();
vi.mocked(isEffectivelyOffline).mockReturnValue(false);
});
describe('useStayPin', () => {
it('FE-STAY-PIN-001: looks the stay up by name and address and says where it found it', async () => {
vi.mocked(findOnMap).mockResolvedValueOnce(SPOT);
const { result } = renderHook(() => useStayPin());
expect(result.current.pin).toEqual({ status: 'idle' });
act(() => result.current.lookUp('Hotel Gracery', '1-19-1 Kabukicho'));
expect(result.current.pin).toEqual({ status: 'searching' });
await waitFor(() => expect(result.current.pin).toEqual({ status: 'found', match: SPOT }));
expect(result.current.match).toEqual(SPOT);
expect(findOnMap).toHaveBeenCalledWith(
['Hotel Gracery 1-19-1 Kabukicho', 'Hotel Gracery', '1-19-1 Kabukicho'],
'en'
);
});
it('FE-STAY-PIN-002: nothing found, a failed search and an empty name all read as not on the map', async () => {
const { result } = renderHook(() => useStayPin());
vi.mocked(findOnMap).mockResolvedValueOnce(null);
act(() => result.current.lookUp('Ryokan', null));
await waitFor(() => expect(result.current.pin).toEqual({ status: 'missing' }));
expect(result.current.match).toBeNull();
vi.mocked(findOnMap).mockRejectedValueOnce(new Error('down'));
act(() => result.current.lookUp('Ryokan', null));
await waitFor(() => expect(result.current.pin).toEqual({ status: 'missing' }));
act(() => result.current.lookUp('', ''));
expect(result.current.pin).toEqual({ status: 'missing' });
});
it('FE-STAY-PIN-003: a later lookup or a hand-picked spot wins over an answer still on its way', async () => {
let answer: (m: typeof SPOT | null) => void = () => undefined;
vi.mocked(findOnMap).mockImplementationOnce(() => new Promise((resolve) => (answer = resolve)));
const { result } = renderHook(() => useStayPin());
act(() => result.current.lookUp('Hotel', 'Somewhere'));
const picked = { lat: 1, lng: 2, label: 'Picked' };
act(() => result.current.pick(picked));
await act(async () => answer(SPOT));
expect(result.current.pin).toEqual({ status: 'found', match: picked });
act(() => result.current.reset());
expect(result.current.pin).toEqual({ status: 'idle' });
});
it('FE-STAY-PIN-004: offline there is nothing to ask, so the review shows nothing', () => {
vi.mocked(isEffectivelyOffline).mockReturnValue(true);
const { result } = renderHook(() => useStayPin());
act(() => result.current.lookUp('Hotel', 'Somewhere'));
expect(result.current.pin).toEqual({ status: 'idle' });
expect(findOnMap).not.toHaveBeenCalled();
});
});
@@ -0,0 +1,69 @@
import { useEffect, useRef, useState } from 'react';
import { usePlaceLanguage } from '../../hooks/usePlaceLanguage';
import { findOnMap, type MapMatch, stayQueries } from '../../repo/mapMatchRepo';
import { isEffectivelyOffline } from '../../sync/networkMode';
/**
* Where an imported stay will sit on the map, as the review shows it: not asked
* yet (or offline, where there is nothing to ask), being looked up, found, or
* not found at all.
*/
export type StayPin =
{ status: 'idle' } | { status: 'searching' } | { status: 'found'; match: MapMatch } | { status: 'missing' };
/**
* The map spot of a stay reviewed from an import, looked up before it is saved
* so the review can say whether it will show on the map. A stay the search did
* not find used to be saved without coordinates and simply never appeared.
* A later lookup always wins over an earlier one still on its way.
*/
export function useStayPin() {
const lang = usePlaceLanguage();
const [pin, setPin] = useState<StayPin>({ status: 'idle' });
const ticket = useRef(0);
useEffect(
() => () => {
ticket.current++;
},
[]
);
const lookUp = (name: string, address: string | null | undefined) => {
const mine = ++ticket.current;
const queries = stayQueries(name, address);
if (isEffectivelyOffline()) {
setPin({ status: 'idle' });
return;
}
if (queries.length === 0) {
setPin({ status: 'missing' });
return;
}
setPin({ status: 'searching' });
findOnMap(queries, lang).then(
(match) => {
if (mine === ticket.current) setPin(match ? { status: 'found', match } : { status: 'missing' });
},
() => {
if (mine === ticket.current) setPin({ status: 'missing' });
}
);
};
return {
pin,
/** The spot to save the stay at, once one is known. */
match: pin.status === 'found' ? pin.match : null,
lookUp,
/** A spot the reader picked by hand, which beats any lookup. */
pick: (match: MapMatch) => {
ticket.current++;
setPin({ status: 'found', match });
},
reset: () => {
ticket.current++;
setPin({ status: 'idle' });
},
};
}