fix(planner): never link an offline temp place id to a hotel booking

The envelope fix routes the created place through the store, which offline
mints a NEGATIVE temp id: reservations have no offline write path, the
mutation queue rewrites temp ids in a URL but never inside another entity's
body, and day_accommodations.place_id carries a foreign key. So a
forced-offline save (the Settings toggle sets the flag while the network is
still live) reached the server with place_id: -1758..., failed the FK inside
the insert transaction and rolled the whole booking back into a 500 — where
dev merely saved it unlinked. Worse, the temp place is in `places` by then,
so every retry matched the same negative id and the booking stayed unsavable
for the rest of the session.

Offline the function now links nothing at all and skips the geocode too: the
booking cannot be written either way, so minting a place would only leave an
orphan behind on the next flush. The name-match arm rejects temp ids for the
same reason. `as never` is gone from the addPlace argument, and
placesApi.create finally declares its `{ place }` return type — an untyped
`r.data` is what let the envelope be misread in the first place.
This commit is contained in:
Maurice
2026-09-05 12:39:22 +02:00
committed by Maurice
parent 6953ede0e3
commit 3e532aab99
3 changed files with 53 additions and 4 deletions
+4 -1
View File
@@ -432,7 +432,10 @@ export const daysApi = {
export const placesApi = {
list: (tripId: number | string, params?: Record<string, unknown>) => apiClient.get(`/trips/${tripId}/places`, { params }).then(r => r.data),
create: (tripId: number | string, data: PlaceCreateRequest) => apiClient.post(`/trips/${tripId}/places`, data).then(r => r.data),
// Typed: an untyped `r.data` is what let `{ place }` be read as a bare place,
// so every hotel booking minted an unlinked duplicate (#2243).
create: (tripId: number | string, data: PlaceCreateRequest): Promise<{ place: Place }> =>
apiClient.post(`/trips/${tripId}/places`, data).then(r => r.data),
get: (tripId: number | string, id: number | string) => apiClient.get(`/trips/${tripId}/places/${id}`).then(r => r.data),
update: (tripId: number | string, id: number | string, data: PlaceUpdateRequest) => apiClient.put(`/trips/${tripId}/places/${id}`, data).then(r => r.data),
delete: (tripId: number | string, id: number | string) => apiClient.delete(`/trips/${tripId}/places/${id}`).then(r => r.data),
@@ -1603,6 +1603,42 @@ describe('useTripPlanner — bookings and transports', () => {
expect(second.create_accommodation.place_id).toBe(901)
})
it('FE-TP-HOOK-081d: offline, no place is minted for a booking that cannot be written', async () => {
env.forcedOffline = true
seedTrip()
const { result } = await renderPlanner()
await act(async () => {
await result.current.handleSaveReservation({
title: 'Ryokan Sakura', type: 'hotel',
create_accommodation: { venue: { name: 'Ryokan Sakura' } },
} as never)
})
// A place created offline gets a negative temp id, and the reservation write
// is online-only — linking one is a foreign-key failure on the server.
expect(actions.addPlace).not.toHaveBeenCalled()
expect(mapsApi.search).not.toHaveBeenCalled()
const payload = actions.addReservation.mock.calls[0][1] as { create_accommodation: { place_id?: number } }
expect(payload.create_accommodation.place_id).toBeUndefined()
})
it('FE-TP-HOOK-081e: a place still holding an offline temp id is not linked as the accommodation', async () => {
actions.addPlace.mockResolvedValue(buildPlace({ id: 902, name: 'Ryokan Sakura' }))
seedTrip({ places: [buildPlace({ id: -1758000000000, name: 'Ryokan Sakura' })] })
const { result } = await renderPlanner()
await act(async () => {
await result.current.handleSaveReservation({
title: 'Ryokan Sakura', type: 'hotel',
create_accommodation: { venue: { name: 'Ryokan Sakura' } },
} as never)
})
const payload = actions.addReservation.mock.calls[0][1] as { create_accommodation: { place_id?: number } }
expect(payload.create_accommodation.place_id).toBe(902)
})
it('FE-TP-HOOK-081b: a geocoded venue without a reviewed address adopts the hit address', async () => {
vi.mocked(mapsApi.search).mockResolvedValue({
places: [{ lat: 35.1, lng: 135.7, address: 'Kyoto, JP' }],
+13 -3
View File
@@ -916,8 +916,18 @@ export function useTripPlanner() {
if (n) {
const existing = places.find(p => p.name?.trim().toLowerCase() === n)
?? places.find(p => p.name && (p.name.toLowerCase().includes(n) || n.includes(p.name.toLowerCase())))
if (existing) return existing.id
// Only a server-side id may be linked. A negative id is an offline temp id
// (mutationQueue.nextTempId): the reservation write is online-only, the queue
// rewrites temp ids in a URL but never inside another entity's body, and
// day_accommodations.place_id carries a foreign key — so a temp id here is a
// rolled-back insert and a 500 instead of a saved booking.
if (existing && existing.id > 0) return existing.id
}
// Offline the booking itself cannot be written (reservations are online-only),
// so minting a place here would only leave an orphan behind on the next flush
// — and its temp id could never be linked anyway. Link nothing, and skip the
// geocode round-trip too; the retry online matches this venue by name.
if (isEffectivelyOffline()) return null
let lat: number | null = null
let lng: number | null = null
let address: string | null = venue.address ?? null
@@ -939,8 +949,8 @@ export function useTripPlanner() {
// learned about the previous one and the name match above could not
// find it. addPlace unwraps the response and puts the place into
// `places`, so the next save reuses it.
const place = await tripActions.addPlace(tripId, { name: name || address || 'Accommodation', lat, lng, address } as never)
return place?.id ?? null
const place = await tripActions.addPlace(tripId, { name: name || address || 'Accommodation', lat, lng, address })
return place && place.id > 0 ? place.id : null
} catch { return null }
}