diff --git a/client/src/pages/tripPlanner/usePlaceEdits.test.ts b/client/src/pages/tripPlanner/usePlaceEdits.test.ts new file mode 100644 index 000000000..573afd207 --- /dev/null +++ b/client/src/pages/tripPlanner/usePlaceEdits.test.ts @@ -0,0 +1,297 @@ +// FE-TP-PLACEEDITS-001 to FE-TP-PLACEEDITS-012 +// +// Writes to the trip's places, driven straight through usePlaceEdits. The store holds +// the places and day lists the delete and category handlers capture for their undo, +// and every write goes to a recorded stand-in for the store's actions, so each case +// can read what was written and what the undo would put back. +import { act, renderHook } from '@testing-library/react' +import { assignmentsApi } from '../../api/client' +import { useTripStore, type TripStoreState } from '../../store/tripStore' +import { resetAllStores } from '../../../tests/helpers/store' +import { buildAssignment, buildDay, buildPlace, buildReservation, buildTrip } from '../../../tests/helpers/factories' +import type { Accommodation } from '../../types' +import type { Can, Translate } from './plannerTypes' +import type { RoadtripFeed } from './useRoadtripFeed' +import { usePlaceEdits } from './usePlaceEdits' + +type Options = Parameters[0] +type Undo = () => Promise + +const toast = { success: vi.fn(), error: vi.fn(), warning: vi.fn(), info: vi.fn() } +// The parameters ride along in the text, so a case can read which names a sentence got. +const t = ((key: string, params?: Record) => (params ? `${key} ${JSON.stringify(params)}` : key)) as Translate +let allowed = true +const can: Can = vi.fn(() => allowed) +const trip = buildTrip({ id: 42 }) + +const lake = buildPlace({ id: 1, name: 'Lake', category_id: 3 }) +const castle = buildPlace({ id: 2, name: 'Castle', category_id: null }) +const summit = buildPlace({ id: 3, name: 'Summit', tour_place_id: 3 }) +const stay = (over: Partial) => ({ id: 70, trip_id: 42, start_day_id: 10, end_day_id: 11, ...over }) as Accommodation + +function makeActions() { + return { + updatePlace: vi.fn(async () => undefined), + addPlace: vi.fn(async (): Promise<{ id: number } | null> => ({ id: 900 })), + assignPlaceToDay: vi.fn(async () => ({ id: 555 })), + refreshDays: vi.fn(async () => undefined), + addFile: vi.fn(async () => undefined), + deletePlace: vi.fn(async (): Promise<{ tourPlaceIds?: number[] } | undefined> => undefined), + deletePlacesMany: vi.fn(async (): Promise<{ tourPlaceIds?: number[] } | undefined> => undefined), + updatePlacesMany: vi.fn(async () => undefined), + } +} + +let actions: ReturnType +let vias: { byDay: Record; reanchor: ReturnType } +let fixed: Options + +function renderEdits(over: Partial = {}) { + const options = { ...fixed, ...over } + return renderHook(() => usePlaceEdits(options)) +} + +const lastUndo = (): Undo => vi.mocked(fixed.pushUndo).mock.lastCall![1] as Undo + +beforeEach(() => { + vi.restoreAllMocks() + vi.clearAllMocks() + resetAllStores() + allowed = true + actions = makeActions() + vias = { byDay: { 10: [{ id: 7, after_order_index: 0, lat: 50, lng: 10.5 }] }, reanchor: vi.fn(async () => undefined) } + useTripStore.setState({ + places: [lake, castle, summit], + days: [buildDay({ id: 10 }), buildDay({ id: 11 })], + assignments: { + 10: [buildAssignment({ id: 101, day_id: 10, place: lake, order_index: 2 })], + 11: [buildAssignment({ id: 111, day_id: 11, place: castle, order_index: 0 })], + 12: [buildAssignment({ id: 121, day_id: 12, place: lake, order_index: 1 })], + }, + }) + fixed = { + tripId: 42, trip, can, toast, t, + tripActions: { ...useTripStore.getState(), ...actions } as unknown as TripStoreState, + places: [lake, castle, summit], allPlaces: [lake, castle, summit], tripAccommodations: [], reservations: [], + toursEnabled: true, tourPlaceIds: new Set(), invalidateTourPlaceIds: vi.fn(), reloadTourPlaceIds: vi.fn(async () => undefined), + selectedPlaceId: null, setSelectedPlaceId: vi.fn(), selectedDayId: 10, + updateRouteForDay: vi.fn(async () => undefined), pushUndo: vi.fn(), forgetPlace: vi.fn(), + editingPlace: null, editingAssignmentId: null, placeFormDayId: null, placeFormPosition: null, + roadtripRoutes: { days: [] } as unknown as RoadtripFeed['roadtripRoutes'], + roadtripVias: vias as unknown as RoadtripFeed['roadtripVias'], + viaLiesBefore: () => via => via.lng < 10.5, + } +}) + +describe('usePlaceEdits', () => { + it('FE-TP-PLACEEDITS-001: the delete question names the booked nights at stake, and only those', () => { + const { result } = renderEdits({ + tripAccommodations: [ + stay({ id: 70, place_id: 1 }), + // A stay whose place is not loaded is named by the stay, and one with no name at all is left out. + stay({ id: 71, place_id: 4, place_name: 'Old Mill' }), + stay({ id: 72, place_id: 5 }), + ], + reservations: [buildReservation({ id: 80, accommodation_id: 70, title: 'Lake' })], + }) + expect(result.current.deletePlaceNote).toBeNull() + expect(result.current.deletePlacesNote).toBeNull() + + act(() => { result.current.setDeletePlaceId(1) }) + expect(result.current.deletePlaceNote).toBe('trip.confirm.deletePlaceBookedSame {"name":"Lake"}') + + act(() => { result.current.setDeletePlaceIds([4, 5]) }) + expect(result.current.deletePlacesNote).toBe('trip.confirm.deletePlaceNight {"name":"Old Mill"}') + + act(() => { result.current.setDeletePlaceIds([1, 4]) }) + expect(result.current.deletePlacesNote).toBe('trip.confirm.deletePlaceBookedSame {"name":"Lake, Old Mill"}') + + act(() => { result.current.setDeletePlaceId(2) }) + expect(result.current.deletePlaceNote).toBeNull() + }) + + it('FE-TP-PLACEEDITS-002: a booking named otherwise is quoted beside the place', () => { + const { result } = renderEdits({ + tripAccommodations: [stay({ id: 70, place_id: 1, reservation_title: 'Lakeside Inn' })], + }) + + act(() => { result.current.setDeletePlaceId(1) }) + + expect(result.current.deletePlaceNote).toBe('trip.confirm.deletePlaceBooked {"name":"Lake","booking":"Lakeside Inn"}') + }) + + it('FE-TP-PLACEEDITS-003: a tour is told apart from a place, by its own id or by the tours list', () => { + const { result } = renderEdits({ tourPlaceIds: new Set([2]) }) + + expect(result.current.isTourPlace(3)).toBe(true) + expect(result.current.isTourPlace(2)).toBe(true) + expect(result.current.isTourPlace(1)).toBe(false) + expect(result.current.deletePlaceIsTour).toBe(false) + expect(result.current.deletePlacesIncludeTours).toBe(false) + + act(() => { result.current.setDeletePlaceId(3) }) + act(() => { result.current.setDeletePlaceIds([1, 2]) }) + expect(result.current.deletePlaceIsTour).toBe(true) + expect(result.current.deletePlacesIncludeTours).toBe(true) + }) + + it('FE-TP-PLACEEDITS-004: deleting asks only for a place, and deleting a tour only for a tour', () => { + const { result } = renderEdits() + + act(() => { result.current.handleDeletePlace(3) }) + expect(result.current.deletePlaceId).toBeNull() + act(() => { result.current.handleDeleteTour(1) }) + expect(result.current.deletePlaceId).toBeNull() + + act(() => { result.current.handleDeleteTour(3) }) + expect(result.current.deletePlaceId).toBe(3) + act(() => { result.current.handleDeletePlace(1) }) + expect(result.current.deletePlaceId).toBe(1) + }) + + it('FE-TP-PLACEEDITS-005: a reader is asked nothing, and with tours off a tour is deleted like a place', () => { + allowed = false + const reader = renderEdits() + act(() => { reader.result.current.handleDeletePlace(1) }) + act(() => { reader.result.current.handleDeleteTour(3) }) + expect(reader.result.current.deletePlaceId).toBeNull() + reader.unmount() + + allowed = true + const { result } = renderEdits({ toursEnabled: false }) + act(() => { result.current.handleDeleteTour(3) }) + expect(result.current.deletePlaceId).toBeNull() + act(() => { result.current.handleDeletePlace(3) }) + expect(result.current.deletePlaceId).toBe(3) + }) + + it('FE-TP-PLACEEDITS-006: an edited place keeps its times and note on the visit, and its files land on it', async () => { + const updateTime = vi.spyOn(assignmentsApi, 'updateTime').mockResolvedValue({} as never) + const updateNotes = vi.spyOn(assignmentsApi, 'updateNotes').mockResolvedValue({} as never) + actions.addFile.mockRejectedValueOnce(new Error('files.tooLarge')).mockResolvedValueOnce(undefined) + const file = new File(['x'], 'a.pdf') + const { result } = renderEdits({ editingPlace: lake, editingAssignmentId: 101 }) + + let saved: unknown + await act(async () => { + saved = await result.current.handleSavePlace({ + name: 'Lake', place_time: '09:00', end_time: '', assignment_notes: '', _pendingFiles: [file, file], + }) + }) + + expect(saved).toEqual({ id: 1 }) + expect(actions.updatePlace).toHaveBeenCalledWith(42, 1, { name: 'Lake' }) + expect(updateTime).toHaveBeenCalledWith(42, 101, { place_time: '09:00', end_time: null }) + expect(updateNotes).toHaveBeenCalledWith(42, 101, { notes: null }) + expect(actions.refreshDays).toHaveBeenCalledWith(42) + expect(actions.addFile).toHaveBeenCalledTimes(2) + expect(toast.error).toHaveBeenCalledWith('files.uploadError') + expect(toast.success).toHaveBeenCalledWith('trip.toast.placeUpdated') + }) + + it('FE-TP-PLACEEDITS-007: a place edited from the pool touches no visit and leaves an untouched note alone', async () => { + const updateTime = vi.spyOn(assignmentsApi, 'updateTime').mockResolvedValue({} as never) + const { result } = renderEdits({ editingPlace: castle }) + + await act(async () => { await result.current.handleSavePlace({ name: 'Castle', place_time: '10:00' }) }) + + expect(actions.updatePlace).toHaveBeenCalledWith(42, 2, { name: 'Castle' }) + expect(updateTime).not.toHaveBeenCalled() + expect(actions.refreshDays).not.toHaveBeenCalled() + }) + + it('FE-TP-PLACEEDITS-008: a service stop the drawn card does not show still lands on the day and position it named', async () => { + const { result } = renderEdits() + + let saved: unknown + await act(async () => { + saved = await result.current.handleSavePlace({ name: 'Charger', lat: 50, lng: 11, _serviceStop: { dayId: 11, position: 2 } }) + }) + + expect(saved).toEqual({ id: 900 }) + expect(actions.assignPlaceToDay).toHaveBeenCalledWith(42, 11, 900, 2) + expect(fixed.updateRouteForDay).toHaveBeenCalledWith(11) + expect(fixed.pushUndo).toHaveBeenCalledWith('undo.addPlace', expect.any(Function)) + await act(async () => { await lastUndo()() }) + expect(actions.deletePlace).toHaveBeenCalledWith(42, 900) + }) + + it('FE-TP-PLACEEDITS-009: a new place at a position carries the vias behind it along, and a failed day link is reported', async () => { + const { result } = renderEdits({ placeFormDayId: 10, placeFormPosition: 1 }) + + await act(async () => { await result.current.handleSavePlace({ name: 'Pump', lat: 50, lng: 11 }) }) + // The via on the far side of the new stop now follows it. + expect(vias.reanchor).toHaveBeenCalledWith(10, { vias: [{ id: 7, after_order_index: 1 }], remove: [] }) + + actions.assignPlaceToDay.mockRejectedValueOnce('nope') + await act(async () => { await result.current.handleSavePlace({ name: 'Pump', lat: 50, lng: 11 }) }) + expect(toast.error).toHaveBeenCalledWith('common.unknownError') + // The place itself was saved, and says so. + expect(toast.success).toHaveBeenLastCalledWith('trip.toast.placeAdded') + }) + + it('FE-TP-PLACEEDITS-010: a place the store could not add is not offered for undo', async () => { + actions.addPlace.mockResolvedValueOnce(null) + const { result } = renderEdits({ placeFormDayId: 10 }) + + let saved: unknown = 'unset' + await act(async () => { saved = await result.current.handleSavePlace({ name: 'Ghost', _pendingFiles: [new File(['x'], 'a.pdf')] }) }) + + expect(saved).toBeUndefined() + expect(actions.assignPlaceToDay).not.toHaveBeenCalled() + expect(actions.addFile).not.toHaveBeenCalled() + expect(fixed.pushUndo).not.toHaveBeenCalled() + }) + + it('FE-TP-PLACEEDITS-011: deleting a place offers it back on the days that still exist, and a place already gone offers nothing', async () => { + const { result } = renderEdits({ selectedPlaceId: 1 }) + act(() => { result.current.setDeletePlaceId(1) }) + + let deleted: unknown + await act(async () => { deleted = await result.current.confirmDeletePlace() }) + expect(deleted).toBeNull() + expect(fixed.setSelectedPlaceId).toHaveBeenCalledWith(null) + expect(fixed.reloadTourPlaceIds).toHaveBeenCalled() + expect(fixed.updateRouteForDay).toHaveBeenCalledWith(10) + await act(async () => { await lastUndo()() }) + expect(actions.addPlace).toHaveBeenCalledWith(42, expect.objectContaining({ name: 'Lake', category_id: 3 })) + // Day 12 is gone, so only day 10 takes it back. + expect(actions.assignPlaceToDay).toHaveBeenCalledTimes(1) + expect(actions.assignPlaceToDay).toHaveBeenCalledWith(42, 10, 900, 2) + + vi.mocked(fixed.pushUndo).mockClear() + act(() => { result.current.setDeletePlaceId(99) }) + await act(async () => { await result.current.confirmDeletePlace() }) + expect(fixed.pushUndo).not.toHaveBeenCalled() + + actions.deletePlace.mockRejectedValueOnce('nope') + await act(async () => { deleted = await result.current.confirmDeletePlace() }) + expect(deleted).toBeNull() + expect(toast.error).toHaveBeenLastCalledWith('common.unknownError') + }) + + it('FE-TP-PLACEEDITS-012: bulk deletes and category moves undo by group, and report a failure whatever was thrown', async () => { + const { result } = renderEdits() + + await act(async () => { await result.current.confirmDeletePlaces([98, 99]) }) + expect(actions.deletePlacesMany).toHaveBeenCalledWith(42, [98, 99]) + expect(fixed.pushUndo).not.toHaveBeenCalled() + + actions.deletePlacesMany.mockRejectedValueOnce('nope') + await act(async () => { await result.current.confirmDeletePlaces([1]) }) + expect(toast.error).toHaveBeenLastCalledWith('common.unknownError') + + await act(async () => { await result.current.confirmChangeCategory([98], 4) }) + expect(actions.updatePlacesMany).toHaveBeenCalledWith(42, [98], { category_id: 4 }) + expect(fixed.pushUndo).not.toHaveBeenCalled() + + await act(async () => { await result.current.confirmChangeCategory([1, 2], 4) }) + await act(async () => { await lastUndo()() }) + expect(actions.updatePlacesMany).toHaveBeenCalledWith(42, [1], { category_id: 3 }) + expect(actions.updatePlacesMany).toHaveBeenCalledWith(42, [2], { category_id: null }) + + actions.updatePlacesMany.mockRejectedValueOnce('nope') + await act(async () => { await result.current.confirmChangeCategory([1], 4) }) + expect(toast.error).toHaveBeenLastCalledWith('common.unknownError') + }) +}) diff --git a/client/src/pages/tripPlanner/usePlaceEdits.ts b/client/src/pages/tripPlanner/usePlaceEdits.ts new file mode 100644 index 000000000..f8c78d757 --- /dev/null +++ b/client/src/pages/tripPlanner/usePlaceEdits.ts @@ -0,0 +1,322 @@ +import { useCallback, useMemo, useState } from 'react' +import { assignmentsApi } from '../../api/client' +import { roadtripInsertion } from '../../components/Roadtrip/dayWindow' +import { reanchorAfterInsert } from '../../components/Roadtrip/roadtripModel' +import type { usePlaceSelection } from '../../hooks/usePlaceSelection' +import type { useRouteCalculation } from '../../hooks/useRouteCalculation' +import type { useTourPlaceIds } from '../../hooks/useTourPlaceIds' +import { translateApiError } from '../../i18n' +import { useTripStore, type TripStoreState } from '../../store/tripStore' +import type { Accommodation, Place } from '../../types' +import type { PlannerBase, PlannerHistory } from './plannerTypes' +import type { PlannerDialogs } from './usePlannerDialogs' +import type { RoadtripFeed } from './useRoadtripFeed' + +interface PlaceEditsOptions + extends Pick, + Pick, + Pick, 'selectedPlaceId' | 'setSelectedPlaceId'>, + Pick, 'tourPlaceIds' | 'invalidateTourPlaceIds' | 'reloadTourPlaceIds'>, + Pick, + Pick { + /** The places the planner lists, without the stops the day lists leave out. */ + places: Place[] + /** Every place of the trip, so a booked night is named whatever the lists leave out. */ + allPlaces: TripStoreState['places'] + tripAccommodations: Accommodation[] + reservations: TripStoreState['reservations'] + toursEnabled: boolean + selectedDayId: TripStoreState['selectedDayId'] + updateRouteForDay: ReturnType['updateRouteForDay'] +} + +/** + * Writes to the trip's places, each with its undo: saving the place form, including the + * day it was added from and the vias that stop pushes along, deleting one place, a tour + * or several places with the booked nights the question warns about, and moving + * several places to another category. + * + * Nothing here runs an effect, so where useTripPlanner calls it changes nothing. + */ +export function usePlaceEdits(options: PlaceEditsOptions) { + const { + tripId, trip, can, tripActions, toast, t, editingPlace, editingAssignmentId, placeFormDayId, placeFormPosition, + places, allPlaces, tripAccommodations, reservations, selectedDayId, selectedPlaceId, setSelectedPlaceId, + toursEnabled, tourPlaceIds, invalidateTourPlaceIds, reloadTourPlaceIds, forgetPlace, pushUndo, updateRouteForDay, + roadtripRoutes, roadtripVias, viaLiesBefore, + } = options + const [deletePlaceId, setDeletePlaceId] = useState(null) + const [deletePlaceIds, setDeletePlaceIds] = useState(null) + const isTourPlace = useCallback((placeId: number) => tourPlaceIds.has(placeId) + || places.some(place => place.id === placeId && place.tour_place_id === placeId), [places, tourPlaceIds]) + const deletePlaceIsTour = deletePlaceId != null && isTourPlace(deletePlaceId) + const deletePlacesIncludeTours = !!deletePlaceIds?.some(isTourPlace) + /** + * The sentence the delete question adds when a night is booked at one of the places. + * + * The server takes a booked night down with its place, and with the night the + * booking made for it and the expense written against that booking. The question + * itself only names the place, and those are the rows the traveller least expects + * to lose, so the dialog says so before the yes. The expense list is loaded with + * the costs tab, not here, so the sentence speaks of any expense rather than + * counting them. A booking named after its hotel, which is how most are named, + * is not quoted a second time. Null when nothing beyond the place is at stake. + */ + const bookedNightsNote = useCallback((placeIds: number[]): string | null => { + const stays = tripAccommodations.filter(stay => stay.place_id != null && placeIds.includes(stay.place_id)) + if (stays.length === 0) return null + const names = [...new Set(stays.map(stay => allPlaces.find(p => p.id === stay.place_id)?.name ?? stay.place_name ?? ''))] + .filter(Boolean) + const bookings = stays + .map(stay => reservations.find(r => r.accommodation_id != null && Number(r.accommodation_id) === stay.id)?.title + ?? stay.reservation_title ?? null) + .filter((title): title is string => !!title) + const name = names.join(', ') + if (bookings.length === 0) return t('trip.confirm.deletePlaceNight', { name }) + return bookings.every(title => names.includes(title)) + ? t('trip.confirm.deletePlaceBookedSame', { name }) + : t('trip.confirm.deletePlaceBooked', { name, booking: bookings.join(', ') }) + }, [tripAccommodations, allPlaces, reservations, t]) + const deletePlaceNote = useMemo( + () => (deletePlaceId ? bookedNightsNote([deletePlaceId]) : null), + [deletePlaceId, bookedNightsNote], + ) + const deletePlacesNote = useMemo( + () => (deletePlaceIds?.length ? bookedNightsNote(deletePlaceIds) : null), + [deletePlaceIds, bookedNightsNote], + ) + + const handleSavePlace = useCallback(async (data) => { + const pendingFiles = data._pendingFiles + delete data._pendingFiles + // Where a service stop added by hand belongs on the drive. The form worked it out + // from the coordinates being saved, because until a place was chosen in it there + // were none to project. + const serviceStop = data._serviceStop + delete data._serviceStop + if (editingPlace) { + // Always strip time fields from place update: time is per-assignment only. + // Same for the day-specific note (#2163): it belongs to the assignment, + // never to the pool place. + const { place_time, end_time, assignment_notes, ...placeData } = data + await tripActions.updatePlace(tripId, editingPlace.id, placeData) + // If editing from assignment context, save time per-assignment + if (editingAssignmentId) { + await assignmentsApi.updateTime(tripId, editingAssignmentId, { place_time: place_time || null, end_time: end_time || null }) + // The form only includes assignment_notes when the user changed it, so + // an untouched note never produces a PUT (#2163). '' clears like null. + if (assignment_notes !== undefined) { + await assignmentsApi.updateNotes(tripId, editingAssignmentId, { notes: assignment_notes || null }) + } + await tripActions.refreshDays(tripId) + } + // Upload pending files with place_id + if (pendingFiles?.length > 0) { + for (const file of pendingFiles) { + const fd = new FormData() + fd.append('file', file) + fd.append('place_id', String(editingPlace.id)) + try { await tripActions.addFile(tripId, fd) } catch (err) { toast.error(translateApiError(t, err, 'files.uploadError')) } + } + } + toast.success(t('trip.toast.placeUpdated')) + return { id: editingPlace.id } + } else { + const place = await tripActions.addPlace(tripId, data) + // A card of the rail can draw stops that are STORED on the day before it + // (`nightSpill.ts`), so a leg named by the card and the position in it is + // translated once here, the same way every other path into this write is. + const card = serviceStop && roadtripRoutes.days.find(day => day.dayId === serviceStop.dayId) + const insert = serviceStop + ? (card && roadtripInsertion(card, serviceStop.position)) || { dayId: serviceStop.dayId, position: serviceStop.position } + : null + const dayId = insert ? insert.dayId : placeFormDayId + const position = insert ? insert.position : placeFormPosition + // Added from inside a day? Then it belongs to that day. Without this the + // place drops into the unplanned pool and, on mobile, into a different + // screen entirely, which reads as "it wasn't saved" (#1998). + if (place?.id && dayId != null) { + // Worked out BEFORE the stop lands, against the day as it stands and the road as + // it is currently driven: once the list has shifted there is no record of which + // leg each via was drawn for. A via is stored as (day, after_order_index) and + // that index is a POSITION in the day's stop list, so a stop dropped into the + // middle of a routed day pushes every via at or behind it onto the wrong leg and + // the drawn road runs forward, doubles back and runs out again. Keyed on the + // position rather than on the service-stop form: a corridor hit handed to the + // full form carries its position too, and was the one way into the middle of a + // day that left the vias where they were. + const plan = position != null && typeof data.lat === 'number' && typeof data.lng === 'number' + ? reanchorAfterInsert( + roadtripVias.byDay[dayId] ?? [], + position, + viaLiesBefore(dayId, { lat: data.lat, lng: data.lng }), + ) + : null + try { + // With a position the stop lands where it will be driven past, not at the end + // of the day. The slice has taken one all along; nothing ever passed it. + await tripActions.assignPlaceToDay(tripId, dayId, place.id, position) + // Awaited before the day re-routes: the routing effect reads the anchors against + // the new stop list, so a correction landing after it would draw the wrong road + // first and the right one a moment later. + if (plan) await roadtripVias.reanchor(dayId, plan) + updateRouteForDay(dayId) + } catch (err: unknown) { + // The place itself exists; only the day link failed. + toast.error(err instanceof Error ? err.message : t('common.unknownError')) + } + } + if (pendingFiles?.length > 0 && place?.id) { + for (const file of pendingFiles) { + const fd = new FormData() + fd.append('file', file) + fd.append('place_id', String(place.id)) + try { await tripActions.addFile(tripId, fd) } catch (err) { toast.error(translateApiError(t, err, 'files.uploadError')) } + } + } + toast.success(t('trip.toast.placeAdded')) + if (place?.id) { + const capturedId = place.id + pushUndo(t('undo.addPlace'), async () => { + await tripActions.deletePlace(tripId, capturedId) + }) + } + // Handed back so the form can link an expense to a place that did not + // exist a moment ago (#1298), the same way the booking modals work. + return place?.id ? { id: place.id } : undefined + } + }, [editingPlace, editingAssignmentId, placeFormDayId, placeFormPosition, roadtripRoutes.days, tripId, toast, pushUndo, updateRouteForDay, roadtripVias, viaLiesBefore]) + + const handleDeletePlace = useCallback( + (placeId) => { + if (!can('place_edit', trip)) return + if (toursEnabled && isTourPlace(placeId)) return + setDeletePlaceId(placeId) + }, + [can, trip, toursEnabled, isTourPlace] + ) + + const handleDeleteTour = useCallback( + (placeId: number) => { + if (!can('place_edit', trip) || !toursEnabled || !isTourPlace(placeId)) return + setDeletePlaceId(placeId) + }, + [can, trip, toursEnabled, isTourPlace] + ) + + const confirmDeletePlace = useCallback(async () => { + if (!deletePlaceId) return null + const state = useTripStore.getState() + const capturedPlace = state.places.find(p => p.id === deletePlaceId) + const capturedAssignments = Object.entries(state.assignments).flatMap(([dayId, as]) => + as.filter(a => a.place?.id === deletePlaceId).map(a => ({ dayId: Number(dayId), orderIndex: a.order_index })) + ) + try { + const deletion = await tripActions.deletePlace(tripId, deletePlaceId) + const deletedTour = Array.isArray(deletion?.tourPlaceIds) + ? deletion.tourPlaceIds.includes(deletePlaceId) + : isTourPlace(deletePlaceId) + if (deletedTour) invalidateTourPlaceIds({ removedPlaceIds: [deletePlaceId] }) + else void reloadTourPlaceIds() + if (selectedPlaceId === deletePlaceId) setSelectedPlaceId(null) + updateRouteForDay(selectedDayId) + toast.success(t('trip.toast.placeDeleted')) + if (deletedTour) forgetPlace(deletePlaceId) + else if (capturedPlace) { + pushUndo(t('undo.deletePlace'), async () => { + const newPlace = await tripActions.addPlace(tripId, { + name: capturedPlace.name, + description: capturedPlace.description, + lat: capturedPlace.lat, + lng: capturedPlace.lng, + address: capturedPlace.address, + category_id: capturedPlace.category_id, + price: capturedPlace.price, + // An undone track has to come back as a track, not a bare point. + route_geometry: capturedPlace.route_geometry, + route_color: capturedPlace.route_color, + }) + const live = new Set(useTripStore.getState().days.map(d => d.id)) + for (const { dayId, orderIndex } of capturedAssignments) { + if (live.has(dayId)) await tripActions.assignPlaceToDay(tripId, dayId, newPlace.id, orderIndex) + } + }) + } + return deletedTour ? deletePlaceId : null + } catch (err: unknown) { + toast.error(err instanceof Error ? err.message : t('common.unknownError')) + return null + } + }, [deletePlaceId, tripId, toast, selectedPlaceId, selectedDayId, updateRouteForDay, pushUndo, forgetPlace, invalidateTourPlaceIds, reloadTourPlaceIds, isTourPlace]) + + const confirmDeletePlaces = useCallback(async (ids?: number[]) => { + const targetIds = ids ?? deletePlaceIds + if (!targetIds?.length) return + const state = useTripStore.getState() + const capturedPlaces = state.places.filter(p => targetIds.includes(p.id)) + const capturedAssignments = Object.entries(state.assignments).flatMap(([dayId, as]) => + as.filter(a => a.place?.id != null && targetIds.includes(a.place.id)).map(a => ({ dayId: Number(dayId), placeId: a.place!.id, orderIndex: a.order_index })) + ) + try { + const deletion = await tripActions.deletePlacesMany(tripId, targetIds) + const deletedTourIds = Array.isArray(deletion?.tourPlaceIds) + ? deletion.tourPlaceIds.filter(id => targetIds.includes(id)) + : targetIds.filter(isTourPlace) + void reloadTourPlaceIds() + if (selectedPlaceId != null && targetIds.includes(selectedPlaceId)) setSelectedPlaceId(null) + if (!ids) setDeletePlaceIds(null) + updateRouteForDay(selectedDayId) + toast.success(t('trip.toast.placesDeleted', { count: capturedPlaces.length })) + if (deletedTourIds.length > 0) deletedTourIds.forEach(forgetPlace) + else if (capturedPlaces.length > 0) { + pushUndo(t('undo.deletePlaces'), async () => { + const live = new Set(useTripStore.getState().days.map(d => d.id)) + for (const place of capturedPlaces) { + const newPlace = await tripActions.addPlace(tripId, { + name: place.name, description: place.description, + lat: place.lat, lng: place.lng, address: place.address, + category_id: place.category_id, price: place.price, + route_geometry: place.route_geometry, route_color: place.route_color, + }) + for (const a of capturedAssignments.filter(x => x.placeId === place.id && live.has(x.dayId))) { + await tripActions.assignPlaceToDay(tripId, a.dayId, newPlace.id, a.orderIndex) + } + } + }) + } + } catch (err: unknown) { toast.error(err instanceof Error ? err.message : t('common.unknownError')) } + }, [deletePlaceIds, tripId, toast, selectedPlaceId, selectedDayId, updateRouteForDay, pushUndo, forgetPlace, reloadTourPlaceIds, isTourPlace]) + + const confirmChangeCategory = useCallback(async (ids: number[], categoryId: number | null) => { + if (!ids.length) return + const state = useTripStore.getState() + // Capture each place's prior category so undo can restore them per group. + const captured = state.places.filter(p => ids.includes(p.id)).map(p => ({ id: p.id, prev: p.category_id ?? null })) + try { + await tripActions.updatePlacesMany(tripId, ids, { category_id: categoryId }) + toast.success(t('places.categoryChanged', { count: ids.length })) + if (captured.length > 0) { + pushUndo(t('undo.changeCategory'), async () => { + // Group the captured ids by their prior category so each set is restored + // in one call ('null' key = previously uncategorized). + const byPrev: Record = {} + for (const { id, prev } of captured) { + const key = prev === null ? 'null' : String(prev) + ;(byPrev[key] ??= []).push(id) + } + for (const [key, group] of Object.entries(byPrev)) { + await tripActions.updatePlacesMany(tripId, group, { category_id: key === 'null' ? null : Number(key) }) + } + }) + } + } catch (err: unknown) { toast.error(err instanceof Error ? err.message : t('common.unknownError')) } + }, [tripId, toast, pushUndo]) + + return { + deletePlaceId, setDeletePlaceId, deletePlaceIds, setDeletePlaceIds, isTourPlace, deletePlaceIsTour, + deletePlacesIncludeTours, deletePlaceNote, deletePlacesNote, handleSavePlace, handleDeletePlace, + handleDeleteTour, confirmDeletePlace, confirmDeletePlaces, confirmChangeCategory, + } +} + +export type PlaceEdits = ReturnType diff --git a/client/src/pages/tripPlanner/useTripPlanner.ts b/client/src/pages/tripPlanner/useTripPlanner.ts index 1639c9e0b..98be08158 100644 --- a/client/src/pages/tripPlanner/useTripPlanner.ts +++ b/client/src/pages/tripPlanner/useTripPlanner.ts @@ -8,8 +8,8 @@ import { getCached, fetchPhoto } from '../../services/photoService' import { useToast } from '../../components/shared/Toast' import { Map, Ticket, PackageCheck, Wallet, FolderOpen, Users, Train, Mountain, Route } from 'lucide-react' import { resolvePluginIcon } from '../../components/shared/PluginIcon' -import { useTranslation, translateApiError } from '../../i18n' -import { addonsApi, accommodationsApi, authApi, tripsApi, assignmentsApi, healthApi, mapsApi, placesApi } from '../../api/client' +import { useTranslation } from '../../i18n' +import { addonsApi, accommodationsApi, authApi, tripsApi, healthApi, mapsApi, placesApi } from '../../api/client' import { getDayOrder } from '../../utils/dayOrder' import { TRANSPORT_TYPES, timedSlot } from '../../utils/dayMerge' import { isOvernightCategory } from '../../components/Roadtrip/stopKinds' @@ -32,7 +32,6 @@ import { stopArrival } from '../../components/Roadtrip/stopArrival' import type { CorridorPoi } from '../../components/Roadtrip/useCorridorPois' import { projectOntoRoute, sliceAtMeters, type LatLng } from '../../components/Roadtrip/corridor' import { - reanchorAfterInsert, reanchorAfterRemove, reanchorByStopOrder, isServiceStopType } from '../../components/Roadtrip/roadtripModel' @@ -67,6 +66,7 @@ import { useRoadtripFeed } from './useRoadtripFeed' import { useRoadtripStopEdits } from './useRoadtripStopEdits' import { useRoadtripAlternatives } from './useRoadtripAlternatives' import { useRoadtripDriveShaping } from './useRoadtripDriveShaping' +import { usePlaceEdits } from './usePlaceEdits' import { useIsPhone } from '../../mobile/useIsPhone' /** @@ -358,46 +358,6 @@ export function useTripPlanner() { const [mobileSidebarOpen, setMobileSidebarOpen] = useState<'left' | 'right' | null>(null) const mobilePlanScrollTopRef = useRef(0) const mobilePlacesScrollTopRef = useRef(0) - const [deletePlaceId, setDeletePlaceId] = useState(null) - const [deletePlaceIds, setDeletePlaceIds] = useState(null) - const isTourPlace = useCallback((placeId: number) => tourPlaceIds.has(placeId) - || places.some(place => place.id === placeId && place.tour_place_id === placeId), [places, tourPlaceIds]) - const deletePlaceIsTour = deletePlaceId != null && isTourPlace(deletePlaceId) - const deletePlacesIncludeTours = !!deletePlaceIds?.some(isTourPlace) - /** - * The sentence the delete question adds when a night is booked at one of the places. - * - * The server takes a booked night down with its place, and with the night the - * booking made for it and the expense written against that booking. The question - * itself only names the place, and those are the rows the traveller least expects - * to lose, so the dialog says so before the yes. The expense list is loaded with - * the costs tab, not here, so the sentence speaks of any expense rather than - * counting them. A booking named after its hotel, which is how most are named, - * is not quoted a second time. Null when nothing beyond the place is at stake. - */ - const bookedNightsNote = useCallback((placeIds: number[]): string | null => { - const stays = tripAccommodations.filter(stay => stay.place_id != null && placeIds.includes(stay.place_id)) - if (stays.length === 0) return null - const names = [...new Set(stays.map(stay => allPlaces.find(p => p.id === stay.place_id)?.name ?? stay.place_name ?? ''))] - .filter(Boolean) - const bookings = stays - .map(stay => reservations.find(r => r.accommodation_id != null && Number(r.accommodation_id) === stay.id)?.title - ?? stay.reservation_title ?? null) - .filter((title): title is string => !!title) - const name = names.join(', ') - if (bookings.length === 0) return t('trip.confirm.deletePlaceNight', { name }) - return bookings.every(title => names.includes(title)) - ? t('trip.confirm.deletePlaceBookedSame', { name }) - : t('trip.confirm.deletePlaceBooked', { name, booking: bookings.join(', ') }) - }, [tripAccommodations, allPlaces, reservations, t]) - const deletePlaceNote = useMemo( - () => (deletePlaceId ? bookedNightsNote([deletePlaceId]) : null), - [deletePlaceId, bookedNightsNote], - ) - const deletePlacesNote = useMemo( - () => (deletePlaceIds?.length ? bookedNightsNote(deletePlaceIds) : null), - [deletePlaceIds, bookedNightsNote], - ) useEffect(() => { // The server runs the import when EITHER kitinerary or the LLM parser is @@ -815,106 +775,16 @@ export function useTripPlanner() { return places.find(p => p.osm_id === stopDraft.poi.osm_id)?.name ?? null }, [stopDraft, places]) - const handleSavePlace = useCallback(async (data) => { - const pendingFiles = data._pendingFiles - delete data._pendingFiles - // Where a service stop added by hand belongs on the drive. The form worked it out - // from the coordinates being saved, because until a place was chosen in it there - // were none to project. - const serviceStop = data._serviceStop - delete data._serviceStop - if (editingPlace) { - // Always strip time fields from place update — time is per-assignment only. - // Same for the day-specific note (#2163): it belongs to the assignment, - // never to the pool place. - const { place_time, end_time, assignment_notes, ...placeData } = data - await tripActions.updatePlace(tripId, editingPlace.id, placeData) - // If editing from assignment context, save time per-assignment - if (editingAssignmentId) { - await assignmentsApi.updateTime(tripId, editingAssignmentId, { place_time: place_time || null, end_time: end_time || null }) - // The form only includes assignment_notes when the user changed it, so - // an untouched note never produces a PUT (#2163). '' clears like null. - if (assignment_notes !== undefined) { - await assignmentsApi.updateNotes(tripId, editingAssignmentId, { notes: assignment_notes || null }) - } - await tripActions.refreshDays(tripId) - } - // Upload pending files with place_id - if (pendingFiles?.length > 0) { - for (const file of pendingFiles) { - const fd = new FormData() - fd.append('file', file) - fd.append('place_id', String(editingPlace.id)) - try { await tripActions.addFile(tripId, fd) } catch (err) { toast.error(translateApiError(t, err, 'files.uploadError')) } - } - } - toast.success(t('trip.toast.placeUpdated')) - return { id: editingPlace.id } - } else { - const place = await tripActions.addPlace(tripId, data) - // A card of the rail can draw stops that are STORED on the day before it - // (`nightSpill.ts`), so a leg named by the card and the position in it is - // translated once here, the same way every other path into this write is. - const card = serviceStop && roadtripRoutes.days.find(day => day.dayId === serviceStop.dayId) - const insert = serviceStop - ? (card && roadtripInsertion(card, serviceStop.position)) || { dayId: serviceStop.dayId, position: serviceStop.position } - : null - const dayId = insert ? insert.dayId : placeFormDayId - const position = insert ? insert.position : placeFormPosition - // Added from inside a day? Then it belongs to that day. Without this the - // place drops into the unplanned pool and, on mobile, into a different - // screen entirely — which reads as "it wasn't saved" (#1998). - if (place?.id && dayId != null) { - // Worked out BEFORE the stop lands, against the day as it stands and the road as - // it is currently driven: once the list has shifted there is no record of which - // leg each via was drawn for. A via is stored as (day, after_order_index) and - // that index is a POSITION in the day's stop list, so a stop dropped into the - // middle of a routed day pushes every via at or behind it onto the wrong leg and - // the drawn road runs forward, doubles back and runs out again. Keyed on the - // position rather than on the service-stop form: a corridor hit handed to the - // full form carries its position too, and was the one way into the middle of a - // day that left the vias where they were. - const plan = position != null && typeof data.lat === 'number' && typeof data.lng === 'number' - ? reanchorAfterInsert( - roadtripVias.byDay[dayId] ?? [], - position, - viaLiesBefore(dayId, { lat: data.lat, lng: data.lng }), - ) - : null - try { - // With a position the stop lands where it will be driven past, not at the end - // of the day. The slice has taken one all along; nothing ever passed it. - await tripActions.assignPlaceToDay(tripId, dayId, place.id, position) - // Awaited before the day re-routes: the routing effect reads the anchors against - // the new stop list, so a correction landing after it would draw the wrong road - // first and the right one a moment later. - if (plan) await roadtripVias.reanchor(dayId, plan) - updateRouteForDay(dayId) - } catch (err: unknown) { - // The place itself exists; only the day link failed. - toast.error(err instanceof Error ? err.message : t('common.unknownError')) - } - } - if (pendingFiles?.length > 0 && place?.id) { - for (const file of pendingFiles) { - const fd = new FormData() - fd.append('file', file) - fd.append('place_id', String(place.id)) - try { await tripActions.addFile(tripId, fd) } catch (err) { toast.error(translateApiError(t, err, 'files.uploadError')) } - } - } - toast.success(t('trip.toast.placeAdded')) - if (place?.id) { - const capturedId = place.id - pushUndo(t('undo.addPlace'), async () => { - await tripActions.deletePlace(tripId, capturedId) - }) - } - // Handed back so the form can link an expense to a place that did not - // exist a moment ago (#1298), the same way the booking modals work. - return place?.id ? { id: place.id } : undefined - } - }, [editingPlace, editingAssignmentId, placeFormDayId, placeFormPosition, roadtripRoutes.days, tripId, toast, pushUndo, updateRouteForDay, roadtripVias, viaLiesBefore]) + const { + handleSavePlace, handleDeletePlace, handleDeleteTour, confirmDeletePlace, confirmDeletePlaces, confirmChangeCategory, + isTourPlace, deletePlaceId, setDeletePlaceId, deletePlaceIds, setDeletePlaceIds, + deletePlaceIsTour, deletePlacesIncludeTours, deletePlaceNote, deletePlacesNote, + } = usePlaceEdits({ + tripId, trip, can, tripActions, toast, t, places, allPlaces, tripAccommodations, reservations, + toursEnabled, tourPlaceIds, invalidateTourPlaceIds, reloadTourPlaceIds, selectedPlaceId, setSelectedPlaceId, + selectedDayId, updateRouteForDay, pushUndo, forgetPlace, + editingPlace, editingAssignmentId, placeFormDayId, placeFormPosition, roadtripRoutes, roadtripVias, viaLiesBefore, + }) // Open the place editor from any entry point (Places pool, inspector, map). // Times live per day-assignment, so when no day is in context resolve the @@ -952,132 +822,6 @@ export function useTripPlanner() { setShowPlaceForm(true) }, [isMobile, isTourPlace, handlePlaceClick, can, trip, assignments, roadtripActive, tripAccommodations, days, overnightOptions, roadtripRoutes.days, setEditingAssignmentId, setEditingPlace, setPlaceFormDayId, setServiceStopForm, setShowPlaceForm, setStopDraft]) - const handleDeletePlace = useCallback( - (placeId) => { - if (!can('place_edit', trip)) return - if (toursEnabled && isTourPlace(placeId)) return - setDeletePlaceId(placeId) - }, - [can, trip, toursEnabled, isTourPlace] - ) - - const handleDeleteTour = useCallback( - (placeId: number) => { - if (!can('place_edit', trip) || !toursEnabled || !isTourPlace(placeId)) return - setDeletePlaceId(placeId) - }, - [can, trip, toursEnabled, isTourPlace] - ) - - const confirmDeletePlace = useCallback(async () => { - if (!deletePlaceId) return null - const state = useTripStore.getState() - const capturedPlace = state.places.find(p => p.id === deletePlaceId) - const capturedAssignments = Object.entries(state.assignments).flatMap(([dayId, as]) => - as.filter(a => a.place?.id === deletePlaceId).map(a => ({ dayId: Number(dayId), orderIndex: a.order_index })) - ) - try { - const deletion = await tripActions.deletePlace(tripId, deletePlaceId) - const deletedTour = Array.isArray(deletion?.tourPlaceIds) - ? deletion.tourPlaceIds.includes(deletePlaceId) - : isTourPlace(deletePlaceId) - if (deletedTour) invalidateTourPlaceIds({ removedPlaceIds: [deletePlaceId] }) - else void reloadTourPlaceIds() - if (selectedPlaceId === deletePlaceId) setSelectedPlaceId(null) - updateRouteForDay(selectedDayId) - toast.success(t('trip.toast.placeDeleted')) - if (deletedTour) forgetPlace(deletePlaceId) - else if (capturedPlace) { - pushUndo(t('undo.deletePlace'), async () => { - const newPlace = await tripActions.addPlace(tripId, { - name: capturedPlace.name, - description: capturedPlace.description, - lat: capturedPlace.lat, - lng: capturedPlace.lng, - address: capturedPlace.address, - category_id: capturedPlace.category_id, - price: capturedPlace.price, - // An undone track has to come back as a track, not a bare point. - route_geometry: capturedPlace.route_geometry, - route_color: capturedPlace.route_color, - }) - const live = new Set(useTripStore.getState().days.map(d => d.id)) - for (const { dayId, orderIndex } of capturedAssignments) { - if (live.has(dayId)) await tripActions.assignPlaceToDay(tripId, dayId, newPlace.id, orderIndex) - } - }) - } - return deletedTour ? deletePlaceId : null - } catch (err: unknown) { - toast.error(err instanceof Error ? err.message : t('common.unknownError')) - return null - } - }, [deletePlaceId, tripId, toast, selectedPlaceId, selectedDayId, updateRouteForDay, pushUndo, forgetPlace, invalidateTourPlaceIds, reloadTourPlaceIds, isTourPlace]) - - const confirmDeletePlaces = useCallback(async (ids?: number[]) => { - const targetIds = ids ?? deletePlaceIds - if (!targetIds?.length) return - const state = useTripStore.getState() - const capturedPlaces = state.places.filter(p => targetIds.includes(p.id)) - const capturedAssignments = Object.entries(state.assignments).flatMap(([dayId, as]) => - as.filter(a => a.place?.id != null && targetIds.includes(a.place.id)).map(a => ({ dayId: Number(dayId), placeId: a.place!.id, orderIndex: a.order_index })) - ) - try { - const deletion = await tripActions.deletePlacesMany(tripId, targetIds) - const deletedTourIds = Array.isArray(deletion?.tourPlaceIds) - ? deletion.tourPlaceIds.filter(id => targetIds.includes(id)) - : targetIds.filter(isTourPlace) - void reloadTourPlaceIds() - if (selectedPlaceId != null && targetIds.includes(selectedPlaceId)) setSelectedPlaceId(null) - if (!ids) setDeletePlaceIds(null) - updateRouteForDay(selectedDayId) - toast.success(t('trip.toast.placesDeleted', { count: capturedPlaces.length })) - if (deletedTourIds.length > 0) deletedTourIds.forEach(forgetPlace) - else if (capturedPlaces.length > 0) { - pushUndo(t('undo.deletePlaces'), async () => { - const live = new Set(useTripStore.getState().days.map(d => d.id)) - for (const place of capturedPlaces) { - const newPlace = await tripActions.addPlace(tripId, { - name: place.name, description: place.description, - lat: place.lat, lng: place.lng, address: place.address, - category_id: place.category_id, price: place.price, - route_geometry: place.route_geometry, route_color: place.route_color, - }) - for (const a of capturedAssignments.filter(x => x.placeId === place.id && live.has(x.dayId))) { - await tripActions.assignPlaceToDay(tripId, a.dayId, newPlace.id, a.orderIndex) - } - } - }) - } - } catch (err: unknown) { toast.error(err instanceof Error ? err.message : t('common.unknownError')) } - }, [deletePlaceIds, tripId, toast, selectedPlaceId, selectedDayId, updateRouteForDay, pushUndo, forgetPlace, reloadTourPlaceIds, isTourPlace]) - - const confirmChangeCategory = useCallback(async (ids: number[], categoryId: number | null) => { - if (!ids.length) return - const state = useTripStore.getState() - // Capture each place's prior category so undo can restore them per group. - const captured = state.places.filter(p => ids.includes(p.id)).map(p => ({ id: p.id, prev: p.category_id ?? null })) - try { - await tripActions.updatePlacesMany(tripId, ids, { category_id: categoryId }) - toast.success(t('places.categoryChanged', { count: ids.length })) - if (captured.length > 0) { - pushUndo(t('undo.changeCategory'), async () => { - // Group the captured ids by their prior category so each set is restored - // in one call ('null' key = previously uncategorized). Map is shadowed by - // the lucide icon import in this file, so use a plain object. - const byPrev: Record = {} - for (const { id, prev } of captured) { - const key = prev === null ? 'null' : String(prev) - ;(byPrev[key] ??= []).push(id) - } - for (const [key, group] of Object.entries(byPrev)) { - await tripActions.updatePlacesMany(tripId, group, { category_id: key === 'null' ? null : Number(key) }) - } - }) - } - } catch (err: unknown) { toast.error(err instanceof Error ? err.message : t('common.unknownError')) } - }, [tripId, toast, pushUndo]) - const handleAssignToDay = useCallback(async (placeId: number, dayId?: number, position?: number) => { const target = dayId || selectedDayId if (!target) { toast.error(t('trip.toast.selectDay')); return false }