diff --git a/client/src/pages/tripPlanner/useTripPlanner.test.tsx b/client/src/pages/tripPlanner/useTripPlanner.test.tsx index 1b199e010..be200d2ed 100644 --- a/client/src/pages/tripPlanner/useTripPlanner.test.tsx +++ b/client/src/pages/tripPlanner/useTripPlanner.test.tsx @@ -13,7 +13,7 @@ import { resetAllStores, seedStore } from '../../../tests/helpers/store' import { buildUser, buildTrip, buildDay, buildPlace, buildAssignment, buildReservation } from '../../../tests/helpers/factories' import { addonsApi, accommodationsApi, authApi, tripsApi, assignmentsApi, - healthApi, airtrailApi, mapsApi, placesApi, + healthApi, airtrailApi, mapsApi, } from '../../api/client' import { accommodationRepo } from '../../repo/accommodationRepo' import { offlineDb } from '../../db/offlineDb' @@ -197,7 +197,6 @@ beforeEach(() => { vi.spyOn(airtrailApi, 'sync').mockResolvedValue({ changed: 0 }) vi.spyOn(mapsApi, 'reverse').mockResolvedValue({ name: '', address: '' } as never) vi.spyOn(mapsApi, 'search').mockResolvedValue({ places: [] } as never) - vi.spyOn(placesApi, 'create').mockResolvedValue({ id: 321 }) vi.mocked(accommodationRepo.list).mockResolvedValue({ accommodations: [] }) vi.mocked(getCached).mockReturnValue(undefined) }) @@ -1533,7 +1532,7 @@ describe('useTripPlanner — bookings and transports', () => { const payload = actions.addReservation.mock.calls[0][1] as { create_accommodation: { place_id?: number; venue?: unknown } } expect(payload.create_accommodation.place_id).toBe(4) expect(payload.create_accommodation.venue).toBeUndefined() - expect(placesApi.create).not.toHaveBeenCalled() + expect(actions.addPlace).not.toHaveBeenCalled() }) it('FE-TP-HOOK-080b: a venue name that only partially matches still links that place', async () => { @@ -1567,9 +1566,41 @@ describe('useTripPlanner — bookings and transports', () => { }) expect(mapsApi.search).toHaveBeenCalledWith('Ryokan Sakura Gion') - expect(placesApi.create).toHaveBeenCalledWith(42, { name: 'Ryokan Sakura', lat: 35.1, lng: 135.7, address: 'Gion' }) - const payload = actions.addReservation.mock.calls[0][1] as { create_accommodation: { place_id?: number } } - expect(payload.create_accommodation.place_id).toBe(321) + // Created through the store (addPlace), which unwraps the API's { place } + // envelope and pushes the place into `places` — not through placesApi + // directly, whose envelope used to be read as the place itself (#2243). + expect(actions.addPlace).toHaveBeenCalledWith(42, { name: 'Ryokan Sakura', lat: 35.1, lng: 135.7, address: 'Gion' }) + const payload = actions.addReservation.mock.calls[0][1] as { create_accommodation: { place_id?: number; venue?: unknown } } + expect(payload.create_accommodation.place_id).toBe(900) + expect(payload.create_accommodation.venue).toBeUndefined() + }) + + it('FE-TP-HOOK-081c: the place created for a new hotel is linked, so the next save of the same hotel reuses it (#2243)', async () => { + vi.mocked(mapsApi.search).mockResolvedValue({ + places: [{ lat: 35.1, lng: 135.7, address: 'Kyoto, JP' }], + } as never) + // The real addPlace puts the created place into the store; mirror that so + // the second save sees it the way the planner does after a create. + actions.addPlace.mockImplementation(async (_tripId: unknown, data: { name: string }) => { + const created = buildPlace({ id: 901, name: data.name, lat: 35.1, lng: 135.7 }) + useTripStore.setState(s => ({ places: [created, ...s.places] })) + return created + }) + seedTrip() + + const { result } = await renderPlanner() + const save = () => result.current.handleSaveReservation({ + title: 'Ryokan Sakura', type: 'hotel', + create_accommodation: { venue: { name: 'Ryokan Sakura' } }, + } as never) + await act(async () => { await save() }) + await act(async () => { await save() }) + + expect(actions.addPlace).toHaveBeenCalledTimes(1) + const first = actions.addReservation.mock.calls[0][1] as { create_accommodation: { place_id?: number } } + const second = actions.addReservation.mock.calls[1][1] as { create_accommodation: { place_id?: number } } + expect(first.create_accommodation.place_id).toBe(901) + expect(second.create_accommodation.place_id).toBe(901) }) it('FE-TP-HOOK-081b: a geocoded venue without a reviewed address adopts the hit address', async () => { @@ -1587,12 +1618,12 @@ describe('useTripPlanner — bookings and transports', () => { }) expect(mapsApi.search).toHaveBeenCalledWith('Ryokan Sakura') - expect(placesApi.create).toHaveBeenCalledWith(42, { name: 'Ryokan Sakura', lat: 35.1, lng: 135.7, address: 'Kyoto, JP' }) + expect(actions.addPlace).toHaveBeenCalledWith(42, { name: 'Ryokan Sakura', lat: 35.1, lng: 135.7, address: 'Kyoto, JP' }) }) it('FE-TP-HOOK-082: a failed geocode still creates the place, a failed create yields no link', async () => { vi.mocked(mapsApi.search).mockRejectedValue(new Error('overpass down')) - vi.mocked(placesApi.create).mockRejectedValue(new Error('quota')) + actions.addPlace.mockRejectedValue(new Error('quota')) seedTrip() const { result } = await renderPlanner() @@ -1603,7 +1634,7 @@ describe('useTripPlanner — bookings and transports', () => { } as never) }) - expect(placesApi.create).toHaveBeenCalled() + expect(actions.addPlace).toHaveBeenCalled() const payload = actions.addReservation.mock.calls[0][1] as { create_accommodation: { place_id?: number } } expect(payload.create_accommodation.place_id).toBeUndefined() }) diff --git a/client/src/pages/tripPlanner/useTripPlanner.ts b/client/src/pages/tripPlanner/useTripPlanner.ts index 7aaae7170..753a8d0f3 100644 --- a/client/src/pages/tripPlanner/useTripPlanner.ts +++ b/client/src/pages/tripPlanner/useTripPlanner.ts @@ -8,7 +8,7 @@ import { useToast } from '../../components/shared/Toast' import { Map, Ticket, PackageCheck, Wallet, FolderOpen, Users, Train } from 'lucide-react' import { resolvePluginIcon } from '../../components/shared/PluginIcon' import { useTranslation, translateApiError } from '../../i18n' -import { addonsApi, accommodationsApi, authApi, tripsApi, assignmentsApi, healthApi, airtrailApi, mapsApi, placesApi } from '../../api/client' +import { addonsApi, accommodationsApi, authApi, tripsApi, assignmentsApi, healthApi, airtrailApi, mapsApi } from '../../api/client' import { parsedItemToDraft, isTransportItem, isUnplaceableItem, type BookingReviewDraft } from '../../components/Planner/parsedItemToDraft' import type { BookingImportPreviewItem } from '@trek/shared' import { accommodationRepo } from '../../repo/accommodationRepo' @@ -933,8 +933,14 @@ export function useTripPlanner() { } } catch { /* geocode failure is non-fatal — create the place without coords */ } try { - const place = await placesApi.create(tripId, { name: name || address || 'Accommodation', lat, lng, address } as never) - return (place as { id?: number })?.id ?? null + // Through the store, not placesApi directly: the API answers { place }, + // and reading .id off that wrapper linked nothing — every save of the + // hotel then minted another orphan place, because the store never + // 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 } catch { return null } }