From 8739b4c74dcf390eaefa5b45f717f91c44547e8e Mon Sep 17 00:00:00 2001 From: elmocito <42518790+elmocito@users.noreply.github.com> Date: Tue, 6 Oct 2026 12:52:25 +0200 Subject: [PATCH] fix(tours): restrict permanent deletion to tours workspace --- .../Planner/DayPlanSidebar.test.tsx | 49 +++++++ .../src/components/Planner/DayPlanSidebar.tsx | 10 +- .../Tours/planner/TourPlannerPanels.test.tsx | 10 ++ .../Tours/planner/TourPlannerPanels.tsx | 33 +++-- .../Tours/planner/useTourPlanner.test.tsx | 36 +++++ .../Tours/planner/useTourPlanner.ts | 22 ++- client/src/hooks/useTourPlaceIds.test.ts | 15 ++ client/src/hooks/useTourPlaceIds.ts | 5 + .../trip/places/MToursSelectionList.test.tsx | 5 +- .../trip/places/MToursSelectionList.tsx | 4 +- client/src/pages/TripPlannerPage.tsx | 14 +- .../src/pages/TripPlannerPage.wiring.test.tsx | 135 ++++++++++++++++++ .../pages/tripPlanner/useTripPlanner.test.tsx | 89 ++++++++++-- .../src/pages/tripPlanner/useTripPlanner.ts | 34 +++-- client/src/store/slices/placesSlice.test.ts | 24 ++++ .../trip/MBrowseActionsSheet.tours.test.tsx | 2 + .../unit/slices/assignmentsSlice.test.ts | 8 +- 17 files changed, 456 insertions(+), 39 deletions(-) diff --git a/client/src/components/Planner/DayPlanSidebar.test.tsx b/client/src/components/Planner/DayPlanSidebar.test.tsx index 78afdf430..58b49957c 100644 --- a/client/src/components/Planner/DayPlanSidebar.test.tsx +++ b/client/src/components/Planner/DayPlanSidebar.test.tsx @@ -1490,6 +1490,55 @@ describe('DayPlanSidebar', () => { expect(onDeletePlace).toHaveBeenCalledWith(42) }) + it('hides permanent Tour Delete from the TRIP-PLAN day card while retaining edit, route, view, and remove-from-day', async () => { + const user = userEvent.setup() + useAddonStore.setState({ addons: [{ id: 'tours', name: 'Tours', type: 'trip', icon: '', enabled: true }] }) + const place = buildPlace({ id: 42, name: 'Ridge walk' }) + const day = buildDay({ id: 10, date: '2025-06-01', title: 'Day 1' }) + const assignment = buildAssignment({ id: 99, day_id: 10, order_index: 0, place }) + const onPlaceClick = vi.fn() + const onRemoveAssignment = vi.fn() + const onDeletePlace = vi.fn() + render( + + ) + + await user.click(screen.getByText('Ridge walk')) + expect(onPlaceClick).toHaveBeenCalled() + fireEvent.contextMenu(screen.getByText('Ridge walk')) + expect(screen.getByText(/Edit/i)).toBeInTheDocument() + expect(screen.getByText(/Leave out of route/i)).toBeInTheDocument() + expect(screen.queryByText(/Delete/i)).not.toBeInTheDocument() + await user.click(screen.getByText(/Remove from day/i)) + expect(onRemoveAssignment).toHaveBeenCalledWith(10, 99) + expect(onDeletePlace).not.toHaveBeenCalled() + }) + + it('TRIP-PLAN preserves legacy route-backed Place Delete when Tours is disabled', async () => { + const user = userEvent.setup() + useAddonStore.setState({ addons: [{ id: 'tours', name: 'Tours', type: 'trip', icon: '', enabled: false }] }) + const place = buildPlace({ id: 43, name: 'Old trail', route_geometry: '[[48,11],[48.01,11.02]]', tour_place_id: 43 }) + const day = buildDay({ id: 10, date: '2025-06-01', title: 'Day 1' }) + const assignment = buildAssignment({ id: 100, day_id: 10, order_index: 0, place }) + const onDeletePlace = vi.fn() + render() + + fireEvent.contextMenu(screen.getByText('Old trail')) + await user.click(screen.getByText(/Delete/i)) + + expect(onDeletePlace).toHaveBeenCalledWith(place.id) + }) + // ── Note card editing (#2249) ───────────────────────── it('FE-PLANNER-DAYPLAN-064: clicking a note row opens its edit modal', async () => { diff --git a/client/src/components/Planner/DayPlanSidebar.tsx b/client/src/components/Planner/DayPlanSidebar.tsx index 4ffdcca4f..74ed8b8f8 100644 --- a/client/src/components/Planner/DayPlanSidebar.tsx +++ b/client/src/components/Planner/DayPlanSidebar.tsx @@ -78,6 +78,7 @@ interface DayPlanSidebarProps { trip: Trip days: Day[] places: Place[] + tourPlaceIds?: ReadonlySet categories: Category[] assignments: AssignmentsMap selectedDayId: number | null @@ -2190,7 +2191,14 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar(props: DayPlanSidebarP safeHttpUrl(place.website) && { label: t('inspector.website'), icon: ExternalLink, onClick: () => window.open(safeHttpUrl(place.website)!, '_blank', 'noopener,noreferrer') }, collectionsEnabled && { label: t('inspector.saveToCollection'), icon: Bookmark, onClick: () => useSaveToCollectionStore.getState().open(placeToSaveTarget(place)) }, { divider: true }, - canEditPlaces && onDeletePlace && { label: t('common.delete'), icon: Trash2, danger: true, onClick: () => onDeletePlace(place.id) }, + canEditPlaces && + onDeletePlace && + !(toursEnabled && (props.tourPlaceIds?.has(place.id) || place.tour_place_id === place.id)) && { + label: t('common.delete'), + icon: Trash2, + danger: true, + onClick: () => onDeletePlace(place.id), + }, ] return ( diff --git a/client/src/components/Tours/planner/TourPlannerPanels.test.tsx b/client/src/components/Tours/planner/TourPlannerPanels.test.tsx index 8333669c6..bebd71ced 100644 --- a/client/src/components/Tours/planner/TourPlannerPanels.test.tsx +++ b/client/src/components/Tours/planner/TourPlannerPanels.test.tsx @@ -124,6 +124,16 @@ describe('TourPlannerToursRail', () => { expect(screen.queryByText('Routing is planning assistance, not a safety guarantee.')).not.toBeInTheDocument() }) + it('exposes permanent deletion in the visible TOUR-PLANNER Tours rail', () => { + const onDeleteTour = vi.fn() + render() + + fireEvent.click(screen.getByRole('button', { name: 'Delete Saved ridge walk' })) + + expect(onDeleteTour).toHaveBeenCalledOnce() + expect(onDeleteTour).toHaveBeenCalledWith(tour.place_id) + }) + it('TOUR-PLANNER-RAIL-002: guards a saved-tour open while the current draft is unsaved', () => { const controller = planner({ hasUnsavedChanges: true }) render() diff --git a/client/src/components/Tours/planner/TourPlannerPanels.tsx b/client/src/components/Tours/planner/TourPlannerPanels.tsx index 82c0dde17..e9690d64d 100644 --- a/client/src/components/Tours/planner/TourPlannerPanels.tsx +++ b/client/src/components/Tours/planner/TourPlannerPanels.tsx @@ -389,9 +389,10 @@ interface TourPlannerToursRailProps extends TourPermissionProps { loading: boolean onAssignToDay: (placeId: number, dayId: number) => void | boolean | Promise onViewGpxTour: (tour: TourListItem) => void + onDeleteTour?: (placeId: number) => void } -export function TourPlannerToursRail({ planner, tours, days, loading, onAssignToDay, onViewGpxTour, canEdit: editPermission, canAssign: assignPermission }: TourPlannerToursRailProps) { +export function TourPlannerToursRail({ planner, tours, days, loading, onAssignToDay, onViewGpxTour, onDeleteTour, canEdit: editPermission, canAssign: assignPermission }: TourPlannerToursRailProps) { const { canEdit, canAssign } = useTourPermissions({ canEdit: editPermission ?? planner.canEdit, canAssign: assignPermission ?? planner.canAssign }) const { t } = useTranslation() const toast = useToast() @@ -496,13 +497,29 @@ export function TourPlannerToursRail({ planner, tours, days, loading, onAssignTo disabled={isSaving} selected={(planner.mode.type === 'edit-saved' || planner.mode.type === 'view-gpx') && planner.mode.placeId === tour.place_id} onSelect={requestOpenTour} - action={canAssign && days.length > 0 ? ( - { if (canAssign) void onAssignToDay(tour.place_id, day.id) }} - /> + action={((canAssign && days.length > 0) || (canEdit && onDeleteTour)) ? ( + <> + {canAssign && days.length > 0 && ( + { if (canAssign) void onAssignToDay(tour.place_id, day.id) }} + /> + )} + {canEdit && onDeleteTour && ( + + )} + ) : undefined} /> ))} diff --git a/client/src/components/Tours/planner/useTourPlanner.test.tsx b/client/src/components/Tours/planner/useTourPlanner.test.tsx index b349b2165..907dcc662 100644 --- a/client/src/components/Tours/planner/useTourPlanner.test.tsx +++ b/client/src/components/Tours/planner/useTourPlanner.test.tsx @@ -1,6 +1,7 @@ // @vitest-environment jsdom import { act, renderHook } from '@testing-library/react' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { render, screen } from '../../../../tests/helpers/render' vi.mock('../../../../../server/src/config', () => { throw new Error('Config initialization is forbidden in client tests') }) vi.mock('../../../../../server/src/db/database', () => { throw new Error('Legacy database imports are forbidden in client tests') }) @@ -20,6 +21,7 @@ vi.mock('../../../api/client', () => ({ toursApi: { create: createTour, detail: vi.mock('../useTourPermissions', () => ({ useTourPermissions: ({ canEdit = true, canAssign = true }: { canEdit?: boolean; canAssign?: boolean }) => ({ canEdit, canAssign }) })) import { useTourPlanner } from './useTourPlanner' +import { TourPlannerRail } from './TourPlannerPanels' const routed = { coordinates: [[48, 11], [48.01, 11.02]] as [number, number][], @@ -1065,4 +1067,38 @@ describe('useTourPlanner', () => { expect(updateTour).not.toHaveBeenCalled() expect(createTour).not.toHaveBeenCalled() }) + + it('clears a successfully deleted saved Tour from the editor and blocks a stale update', async () => { + detailTour.mockResolvedValue({ + tour: savedTour, + waypoints: [ + { lat: 48, lng: 11, role: 'start', sequence: 0 }, + { lat: 48.01, lng: 11.02, role: 'end', sequence: 1 }, + ], + }) + const { result } = renderHook(() => useTourPlanner({ tripId: 29 })) + await act(async () => { expect(await result.current.openTour(savedTour)).toBe(true) }) + act(() => result.current.setName('Unsaved rename')) + await act(async () => { await vi.advanceTimersByTimeAsync(450) }) + expect(result.current.mode).toEqual({ type: 'edit-saved', placeId: savedTour.place_id }) + const view = render() + expect(screen.getByRole('heading', { name: 'Edit tour' })).toBeInTheDocument() + + act(() => result.current.forgetDeletedTour(savedTour.place_id)) + view.rerender() + + expect(result.current.mode).toEqual({ type: 'neutral' }) + expect(result.current.editingPlaceId).toBeNull() + expect(result.current.waypoints).toEqual([]) + expect(result.current.route).toBeNull() + expect(result.current.routeProfileFocus).toBeNull() + expect(result.current.error).toBeNull() + expect(result.current.hasUnsavedChanges).toBe(false) + expect(result.current.canSave).toBe(false) + expect(localStorage.getItem('tour-draft-29')).toBeNull() + expect(screen.getByText('Plan a tour')).toBeInTheDocument() + expect(screen.queryByText('Tour could not be saved. Your draft is still here.')).not.toBeInTheDocument() + await act(async () => { expect(await result.current.save()).toBeNull() }) + expect(updateTour).not.toHaveBeenCalled() + }) }) diff --git a/client/src/components/Tours/planner/useTourPlanner.ts b/client/src/components/Tours/planner/useTourPlanner.ts index f1c994fbe..f8642f419 100644 --- a/client/src/components/Tours/planner/useTourPlanner.ts +++ b/client/src/components/Tours/planner/useTourPlanner.ts @@ -142,6 +142,7 @@ export function useTourPlanner({ tripId, active = true, onSaved, canEdit: editPe const draftRevisionRef = useRef(0) const saveRequestIdRef = useRef(0) const saveInFlightRef = useRef(false) + const deletedTourPlaceIdsRef = useRef(new Set()) const mountedRef = useRef(false) const contextGenerationRef = useRef(0) const detailRequestIdRef = useRef(0) @@ -338,6 +339,20 @@ export function useTourPlanner({ tripId, active = true, onSaved, canEdit: editPe setMapFocusKey(key => key + 1) }, [invalidateDetailRequest, interruptRouting, storageKey]) + const forgetDeletedTour = useCallback((placeId: number) => { + deletedTourPlaceIdsRef.current.add(placeId) + const editorReferencesTour = editingPlaceIdRef.current === placeId + || readOnlyGpxTourRef.current?.tour.place_id === placeId + || detailPlaceIdRef.current === placeId + if (!editorReferencesTour) return + if (saveInFlightRef.current) { + saveRequestIdRef.current += 1 + saveInFlightRef.current = false + setIsSaving(false) + } + returnToNeutral() + }, [returnToNeutral]) + const startNewTour = useCallback((confirmed = false) => { if (!editPermissionRef.current || saveInFlightRef.current) return const currentDraftIsEmpty = editingPlaceId === null && waypoints.length === 0 && name.trim().length === 0 @@ -372,7 +387,7 @@ export function useTourPlanner({ tripId, active = true, onSaved, canEdit: editPe }, [invalidateDetailRequest]) const openTour = useCallback(async (tour: TourListItem) => { - if (!mountedRef.current || !activeRef.current || saveInFlightRef.current) return false + if (!mountedRef.current || !activeRef.current || saveInFlightRef.current || deletedTourPlaceIdsRef.current.has(tour.place_id)) return false detailAbortRef.current?.abort() const controller = new AbortController() detailAbortRef.current = controller @@ -544,6 +559,7 @@ export function useTourPlanner({ tripId, active = true, onSaved, canEdit: editPe const save = useCallback(async () => { if (!mountedRef.current || saveInFlightRef.current || !editPermissionRef.current || !canSave || !hasCompleteElevation(enrichedGeometry)) return null + if (editingPlaceId !== null && deletedTourPlaceIdsRef.current.has(editingPlaceId)) return null invalidateDetailRequest() saveInFlightRef.current = true const requestId = ++saveRequestIdRef.current @@ -586,7 +602,7 @@ export function useTourPlanner({ tripId, active = true, onSaved, canEdit: editPe editingPlaceIdRef.current = result.tour.place_id setSaveOutcome(result.tour) } - if (mountedRef.current && String(tripIdRef.current) === submittedTripId) { + if (mountedRef.current && String(tripIdRef.current) === submittedTripId && !deletedTourPlaceIdsRef.current.has(result.tour.place_id)) { try { await onSaved?.(result) } catch { return result } } return result @@ -615,7 +631,7 @@ export function useTourPlanner({ tripId, active = true, onSaved, canEdit: editPe routeProfileFocus, setRouteProfileFocus, mapBaseLayer, setMapBaseLayer, mode, readOnlyGpxTour, readOnlyGpxAnalysis, viewGpxTour, closeGpxTour, - startNewTour, newTourConfirmationOpen, cancelNewTour, + startNewTour, newTourConfirmationOpen, cancelNewTour, forgetDeletedTour, elevationProfileExpanded, toggleElevationProfile, newDraftGeneration, draftRestored, mapFocusKey, editingPlaceId, openingTourId, openTour, saveOutcome, hasUnsavedChanges, canSave, save, discard, returnToNeutral, diff --git a/client/src/hooks/useTourPlaceIds.test.ts b/client/src/hooks/useTourPlaceIds.test.ts index 113bf90d5..da8f7040e 100644 --- a/client/src/hooks/useTourPlaceIds.test.ts +++ b/client/src/hooks/useTourPlaceIds.test.ts @@ -78,6 +78,21 @@ describe('useTourPlaceIds', () => { expect(result.current.tours).toEqual([edited, tour]) }) + it('removes a deleted Tour from the visible list before the refresh completes', async () => { + const { result } = renderHook(() => useTourPlaceIds(7, true)) + await waitFor(() => expect(result.current.tours).toEqual([tour])) + const refresh = deferred<{ tours: TourListItem[] }>() + vi.mocked(toursApi.list).mockReturnValueOnce(refresh.promise) + + act(() => result.current.invalidateTourPlaceIds({ removedPlaceIds: [tour.place_id] })) + + expect(result.current.tours).toEqual([]) + expect(result.current.tourPlaceIds.size).toBe(0) + await act(async () => refresh.resolve({ tours: [] })) + await waitFor(() => expect(result.current.toursLoading).toBe(false)) + expect(result.current.tourLoadError).toBe(false) + }) + it('shows only the new trip Tours after a successful trip switch', async () => { const tripTwoTour = tourFor(84, 'Trip two tour') vi.mocked(toursApi.list).mockImplementation(async tripId => ({ diff --git a/client/src/hooks/useTourPlaceIds.ts b/client/src/hooks/useTourPlaceIds.ts index e8882dbea..b84dcb9b4 100644 --- a/client/src/hooks/useTourPlaceIds.ts +++ b/client/src/hooks/useTourPlaceIds.ts @@ -54,6 +54,11 @@ export function useTourPlaceIds(tripId: number, enabled: boolean) { || !activeRequestRef.current.enabled) return const added = change?.placeIds ?? [] const removed = change?.removedPlaceIds ?? [] + if (removed.length) { + setTourData(current => current?.tripId === tripId + ? { tripId, tours: current.tours.filter(tour => !removed.includes(tour.place_id)) } + : current) + } if (added.length || removed.length) { setPendingPlaceIds(current => { const ids = new Set(current?.tripId === tripId ? current.ids : []) diff --git a/client/src/mobile/screens/trip/places/MToursSelectionList.test.tsx b/client/src/mobile/screens/trip/places/MToursSelectionList.test.tsx index 1f61a6cc6..baa2b169d 100644 --- a/client/src/mobile/screens/trip/places/MToursSelectionList.test.tsx +++ b/client/src/mobile/screens/trip/places/MToursSelectionList.test.tsx @@ -91,8 +91,8 @@ describe('MToursSelectionList', () => { expect(name).not.toHaveClass('truncate') }) - it('keeps metrics atomic and gives the mobile day action a separate full-width row', () => { - const { container } = render() + it('keeps the mobile TRIP-PLAN Tours browse list limited to viewing and assignment', () => { + const { container } = render() expect(screen.getByRole('option', { name: new RegExp(tour.name) })).toHaveAttribute('aria-selected', 'true') const metrics = screen.getByTestId('mobile-tour-metrics') expect(metrics).toHaveTextContent('12.5 km') @@ -109,6 +109,7 @@ describe('MToursSelectionList', () => { add.click() expect(openSheet).toHaveBeenCalledWith('bract', { placeId: tour.place_id, dayPicker: true }) expect(container.querySelector('[data-testid="mobile-tour-metrics"]')).toBeTruthy() + expect(screen.queryByRole('button', { name: 'Delete' })).not.toBeInTheDocument() }) it('keeps a long German name and source/difficulty labels intact', () => { diff --git a/client/src/mobile/screens/trip/places/MToursSelectionList.tsx b/client/src/mobile/screens/trip/places/MToursSelectionList.tsx index c5d490cd7..c711b7418 100644 --- a/client/src/mobile/screens/trip/places/MToursSelectionList.tsx +++ b/client/src/mobile/screens/trip/places/MToursSelectionList.tsx @@ -22,8 +22,8 @@ interface MToursSelectionListProps extends TourPermissionProps { * (the same one MPlacesBrowser's unplanned-place rows open) — a tour is a * Place under the hood, so no new linking code is needed here either. */ -export default function MToursSelectionList({ planner, shell, filter, canEdit, canAssign: assignPermission }: MToursSelectionListProps) { - const { canAssign } = useTourPermissions({ tripId: planner.tripId, canEdit, canAssign: assignPermission }) +export default function MToursSelectionList({ planner, shell, filter, canAssign: assignPermission }: MToursSelectionListProps) { + const { canAssign } = useTourPermissions({ tripId: planner.tripId, canAssign: assignPermission }) const { t } = planner const distanceUnit = useSettingsStore(s => s.settings.distance_unit) const tours: TourListItem[] = planner.tours diff --git a/client/src/pages/TripPlannerPage.tsx b/client/src/pages/TripPlannerPage.tsx index 729947640..a8396fa28 100644 --- a/client/src/pages/TripPlannerPage.tsx +++ b/client/src/pages/TripPlannerPage.tsx @@ -337,6 +337,7 @@ function TripPlannerPageDesktop(): React.ReactElement | null { route, routeWalking, routeSegments, routeInfo, setRoute, setRouteInfo, updateRouteForDay, handleSelectDay, handlePlaceClick, handleMarkerClick, handleMapClick, handleMapContextMenu, handlePoiClick, handleSavePlace, openPlaceEditor, handleDeletePlace, confirmDeletePlace, confirmDeletePlaces, confirmChangeCategory, + handleDeleteTour, handleAssignToDay, handleMoveToDay, handleRemoveAssignment, handleReorder, handleReorderDays, handleAddDay, dayAdd, handleUpdateDayTitle, deleteDayQuestion, handleDeleteDay, clearDayId, clearDayTitle, handleClearDay, cancelClearDay, confirmClearDay, @@ -698,6 +699,7 @@ function TripPlannerPageDesktop(): React.ReactElement | null { trip={trip} days={days} places={places} + tourPlaceIds={tourPlaceIds} categories={categories} assignments={storedAssignments} selectedDayId={selectedDayId} @@ -1023,7 +1025,6 @@ function TripPlannerPageDesktop(): React.ReactElement | null { onFileUpload={canUploadFiles ? (formData) => tripActions.addFile(tripId, formData) : undefined} onAssignToDay={handleAssignToDay} onRemoveAssignment={handleRemoveAssignment} - onDelete={() => handleDeletePlace(selectedPlace.id)} leftWidth={leftPanelPx} rightWidth={rightPanelPx} /> @@ -1047,7 +1048,6 @@ function TripPlannerPageDesktop(): React.ReactElement | null { onFileUpload={canUploadFiles ? (formData) => tripActions.addFile(tripId, formData) : undefined} onAssignToDay={handleAssignToDay} onRemoveAssignment={handleRemoveAssignment} - onDelete={() => { handleDeletePlace(selectedPlace.id); setSelectedPlaceId(null) }} /> , @@ -1065,8 +1065,8 @@ function TripPlannerPageDesktop(): React.ReactElement | null {
{mobileSidebarOpen === 'left' - ? { handleSelectDay(id); setMobileSidebarOpen(null) }} onPlaceClick={(placeId, assignmentId) => { handlePlaceClick(placeId, assignmentId) }} onReorder={handleReorder} onReorderDays={handleReorderDays} onAddDay={handleAddDay} dayAdd={dayAdd} onDeleteDay={handleDeleteDay} deleteDayQuestion={deleteDayQuestion} onClearDay={can('day_edit', trip) ? handleClearDay : undefined} onUpdateDayTitle={handleUpdateDayTitle} onAssignToDay={handleAssignToDay} onMoveToDay={handleMoveToDay} onRouteCalculated={(r) => { if (r) { setRoute([r.coordinates]); setRouteInfo(r) } else { setRoute(null); setRouteInfo(null) } }} reservations={reservations} visibleConnectionIds={visibleConnections} onToggleConnection={toggleConnection} allConnectionsShown={allConnectionsShown} onToggleAllConnections={toggleAllConnections} onAddReservation={(dayId) => { setEditingReservation(null); tripActions.setSelectedDay(dayId); setShowReservationModal(true); setMobileSidebarOpen(null) }} onAddTransport={can('day_edit', trip) ? (dayId) => { setTransportModalDayId(dayId); setEditingTransport(null); setTransitPrefill(null); setTransportModalAutomated(false); setShowTransportModal(true); setMobileSidebarOpen(null) } : undefined} onOpenTransit={(r) => { setTransitJourney(r); setMobileSidebarOpen(null) }} onPlanTransit={can('day_edit', trip) && tripHasDates ? (dayId) => { setTransportModalDayId(dayId); setEditingTransport(null); setTransitPrefill(null); setTransportModalAutomated(true); setShowTransportModal(true); setMobileSidebarOpen(null) } : undefined} onPlanTransitLeg={can('day_edit', trip) && tripHasDates ? ({ dayId, from, to, time }) => { setTransportModalDayId(dayId); setEditingTransport(null); setTransitPrefill({ from, to, time }); setTransportModalAutomated(true); setShowTransportModal(true); setMobileSidebarOpen(null) } : undefined} onAddPlace={() => { setEditingPlace(null); setPlaceFormDayId(null); setShowPlaceForm(true); setMobileSidebarOpen(null) }} onCreatePlaceForDay={can('place_edit', trip) ? (dayId) => { setEditingPlace(null); setPlaceFormDayId(dayId); setShowPlaceForm(true); setMobileSidebarOpen(null) } : undefined} onDayDetail={(day) => { setShowDayDetail(day); setSelectedPlaceId(null); selectAssignment(null) }} onRemoveAssignment={handleRemoveAssignment} onEditPlace={(place, assignmentId) => { setEditingPlace(place); setEditingAssignmentId(assignmentId || null); setPlaceFormDayId(null); setShowPlaceForm(true); setMobileSidebarOpen(null) }} onDeletePlace={(placeId) => handleDeletePlace(placeId)} accommodations={tripAccommodations} routeShown={routeShown} routeProfile={routeProfile} onToggleRoute={() => setRouteShown(v => !v)} onSetRouteProfile={setRouteProfile} onNavigateToFiles={() => { setMobileSidebarOpen(null); handleTabChange('dateien') }} onExpandedDaysChange={setExpandedDayIds} pushUndo={pushUndo} canUndo={canUndo} lastActionLabel={lastActionLabel} onUndo={handleUndo} onEditTransport={can('day_edit', trip) ? (reservation) => { setEditingTransport(reservation); setTransportModalDayId(reservation.day_id ?? null); setShowTransportModal(true); setMobileSidebarOpen(null) } : undefined} onEditReservation={can('reservation_edit', trip) ? (r) => { setEditingReservation(r); setShowReservationModal(true); setMobileSidebarOpen(null) } : undefined} initialScrollTop={mobilePlanScrollTopRef.current} onScrollTopChange={(top) => { mobilePlanScrollTopRef.current = top }} showRouteToolsWhenExpanded isMobile /> - : handleSelectDay(null)} selectedPlaceId={selectedPlaceId} onPlaceClick={(placeId) => { handlePlaceClick(placeId); setMobileSidebarOpen(null) }} onAddPlace={() => { setEditingPlace(null); setPlaceFormDayId(null); setShowPlaceForm(true); setMobileSidebarOpen(null) }} onAssignToDay={handleAssignToDay} onEditPlace={(place) => { openPlaceEditor(place); setMobileSidebarOpen(null) }} onDeletePlace={(placeId) => handleDeletePlace(placeId)} onBulkDeletePlaces={(ids) => setDeletePlaceIds(ids)} onBulkDeleteConfirm={(ids) => confirmDeletePlaces(ids)} onBulkChangeCategory={(ids, catId) => confirmChangeCategory(ids, catId)} days={days} isMobile pushUndo={pushUndo} initialScrollTop={mobilePlacesScrollTopRef.current} onScrollTopChange={(top) => { mobilePlacesScrollTopRef.current = top }} /> + ? { handleSelectDay(id); setMobileSidebarOpen(null) }} onPlaceClick={(placeId, assignmentId) => { handlePlaceClick(placeId, assignmentId) }} onReorder={handleReorder} onReorderDays={handleReorderDays} onAddDay={handleAddDay} dayAdd={dayAdd} onDeleteDay={handleDeleteDay} deleteDayQuestion={deleteDayQuestion} onClearDay={can('day_edit', trip) ? handleClearDay : undefined} onUpdateDayTitle={handleUpdateDayTitle} onAssignToDay={handleAssignToDay} onMoveToDay={handleMoveToDay} onRouteCalculated={(r) => { if (r) { setRoute([r.coordinates]); setRouteInfo(r) } else { setRoute(null); setRouteInfo(null) } }} reservations={reservations} visibleConnectionIds={visibleConnections} onToggleConnection={toggleConnection} allConnectionsShown={allConnectionsShown} onToggleAllConnections={toggleAllConnections} onAddReservation={(dayId) => { setEditingReservation(null); tripActions.setSelectedDay(dayId); setShowReservationModal(true); setMobileSidebarOpen(null) }} onAddTransport={can('day_edit', trip) ? (dayId) => { setTransportModalDayId(dayId); setEditingTransport(null); setTransitPrefill(null); setTransportModalAutomated(false); setShowTransportModal(true); setMobileSidebarOpen(null) } : undefined} onOpenTransit={(r) => { setTransitJourney(r); setMobileSidebarOpen(null) }} onPlanTransit={can('day_edit', trip) && tripHasDates ? (dayId) => { setTransportModalDayId(dayId); setEditingTransport(null); setTransitPrefill(null); setTransportModalAutomated(true); setShowTransportModal(true); setMobileSidebarOpen(null) } : undefined} onPlanTransitLeg={can('day_edit', trip) && tripHasDates ? ({ dayId, from, to, time }) => { setTransportModalDayId(dayId); setEditingTransport(null); setTransitPrefill({ from, to, time }); setTransportModalAutomated(true); setShowTransportModal(true); setMobileSidebarOpen(null) } : undefined} onAddPlace={() => { setEditingPlace(null); setPlaceFormDayId(null); setShowPlaceForm(true); setMobileSidebarOpen(null) }} onCreatePlaceForDay={can('place_edit', trip) ? (dayId) => { setEditingPlace(null); setPlaceFormDayId(dayId); setShowPlaceForm(true); setMobileSidebarOpen(null) } : undefined} onDayDetail={(day) => { setShowDayDetail(day); setSelectedPlaceId(null); selectAssignment(null) }} onRemoveAssignment={handleRemoveAssignment} onEditPlace={(place, assignmentId) => { setEditingPlace(place); setEditingAssignmentId(assignmentId || null); setPlaceFormDayId(null); setShowPlaceForm(true); setMobileSidebarOpen(null) }} onDeletePlace={(placeId) => handleDeletePlace(placeId)} accommodations={tripAccommodations} routeShown={routeShown} routeProfile={routeProfile} onToggleRoute={() => setRouteShown(v => !v)} onSetRouteProfile={setRouteProfile} onNavigateToFiles={() => { setMobileSidebarOpen(null); handleTabChange('dateien') }} onExpandedDaysChange={setExpandedDayIds} pushUndo={pushUndo} canUndo={canUndo} lastActionLabel={lastActionLabel} onUndo={handleUndo} onEditTransport={can('day_edit', trip) ? (reservation) => { setEditingTransport(reservation); setTransportModalDayId(reservation.day_id ?? null); setShowTransportModal(true); setMobileSidebarOpen(null) } : undefined} onEditReservation={can('reservation_edit', trip) ? (r) => { setEditingReservation(r); setShowReservationModal(true); setMobileSidebarOpen(null) } : undefined} initialScrollTop={mobilePlanScrollTopRef.current} onScrollTopChange={(top) => { mobilePlanScrollTopRef.current = top }} showRouteToolsWhenExpanded isMobile /> + : handleSelectDay(null)} selectedPlaceId={selectedPlaceId} onPlaceClick={(placeId) => { handlePlaceClick(placeId); setMobileSidebarOpen(null) }} onAddPlace={() => { setEditingPlace(null); setPlaceFormDayId(null); setShowPlaceForm(true); setMobileSidebarOpen(null) }} onAssignToDay={handleAssignToDay} onEditPlace={(place) => { openPlaceEditor(place); setMobileSidebarOpen(null) }} onDeletePlace={(placeId) => handleDeletePlace(placeId)} onBulkDeletePlaces={(ids) => setDeletePlaceIds(ids)} onBulkDeleteConfirm={(ids) => confirmDeletePlaces(ids)} onBulkChangeCategory={(ids, catId) => confirmChangeCategory(ids, catId)} days={days} isMobile pushUndo={pushUndo} initialScrollTop={mobilePlacesScrollTopRef.current} onScrollTopChange={(top) => { mobilePlacesScrollTopRef.current = top }} /> }
@@ -1160,6 +1160,7 @@ function TripPlannerPageDesktop(): React.ReactElement | null { days={days} loading={toursLoading} onAssignToDay={(placeId, dayId) => handleAssignToDay(placeId, dayId)} + onDeleteTour={handleDeleteTour} onViewGpxTour={tour => { const place = places.find(item => item.id === tour.place_id) tourPlanner.viewGpxTour(tour, place?.route_geometry ?? null) @@ -1380,7 +1381,10 @@ function TripPlannerPageDesktop(): React.ReactElement | null { setDeletePlaceId(null)} - onConfirm={confirmDeletePlace} + onConfirm={async () => { + const deletedTourId = await confirmDeletePlace() + if (deletedTourId != null) tourPlanner.forgetDeletedTour(deletedTourId) + }} title={t('common.delete')} message={deletePlaceIsTour ? [t('tours.delete.confirmBody'), deletePlaceNote].filter(Boolean).join(' ') diff --git a/client/src/pages/TripPlannerPage.wiring.test.tsx b/client/src/pages/TripPlannerPage.wiring.test.tsx index a749ca6ee..b34f76272 100644 --- a/client/src/pages/TripPlannerPage.wiring.test.tsx +++ b/client/src/pages/TripPlannerPage.wiring.test.tsx @@ -26,6 +26,7 @@ const plannerActions = vi.hoisted(() => ({ viewGpxTour: vi.fn(), addWaypoint: vi.fn(), openTour: vi.fn(), + forgetDeletedTour: vi.fn((_placeId: number) => undefined), mode: { type: 'neutral' } as unknown, isSaving: false, waypoints: [] as unknown, @@ -127,6 +128,7 @@ vi.mock('../components/Tours/planner/useTourPlanner', () => ({ readOnlyGpxTour: plannerActions.readOnlyGpxTour, readOnlyGpxAnalysis: plannerActions.readOnlyGpxAnalysis, discard: vi.fn(), + forgetDeletedTour: plannerActions.forgetDeletedTour, }), })) @@ -383,6 +385,7 @@ function baseState(): HookState { handleSavePlace: vi.fn(async () => undefined), openPlaceEditor: vi.fn(), handleDeletePlace: vi.fn(), + handleDeleteTour: vi.fn(), confirmDeletePlace: vi.fn(async () => undefined), confirmDeletePlaces: vi.fn(async () => undefined), confirmChangeCategory: vi.fn(async () => undefined), @@ -435,6 +438,7 @@ beforeEach(() => { plannerActions.viewGpxTour.mockReset() plannerActions.addWaypoint.mockReset() plannerActions.openTour.mockReset() + plannerActions.forgetDeletedTour.mockReset() plannerActions.mode = { type: 'neutral' } plannerActions.isSaving = false plannerActions.waypoints = [] @@ -621,6 +625,137 @@ describe('TripPlannerPage — shell', () => { expect(screen.queryByTestId('tour-planner-rail')).not.toBeInTheDocument() }) + it('does not offer permanent Tour Delete from the desktop TRIP-PLAN detail', () => { + const tourPlace = buildPlace({ id: 91, name: 'Ridge walk', tour_place_id: 91 }) + const selectedTour = { + place_id: 91, + name: tourPlace.name, + tour_type: 'hike', + distance: 8, + elevation_gain: 420, + elevation_loss: 390, + duration: null, + difficulty: null, + wanderer_ref: null, + match_confidence: null, + tour_group_id: null, + max_hiking_difficulty: 2, + planned: true, + caution: false, + has_waypoints: true, + } + renderPage({ + toursEnabled: true, + selectedPlace: tourPlace, + selectedPlaceId: tourPlace.id, + selectedTour, + }) + expect(props('tourDetail').onDelete).toBeUndefined() + }) + + it('routes visible TOUR-PLANNER Tours-rail deletion through the existing confirmation', async () => { + const savedTour = { + place_id: 91, + name: 'Ridge walk', + tour_type: 'hike', + distance: 8, + elevation_gain: 420, + elevation_loss: 390, + duration: null, + difficulty: null, + wanderer_ref: null, + match_confidence: null, + tour_group_id: null, + max_hiking_difficulty: 2, + planned: false, + caution: false, + has_waypoints: true, + } + const handleDeleteTour = vi.fn((placeId: number) => { + hookState = { ...hookState, deletePlaceId: placeId, deletePlaceIsTour: true } + }) + const setDeletePlaceId = vi.fn((placeId: number | null) => { + hookState = { ...hookState, deletePlaceId: placeId } + }) + const confirmDeletePlace = vi.fn(async (): Promise => savedTour.place_id) + plannerActions.mode = { type: 'edit-saved', placeId: savedTour.place_id } + plannerActions.forgetDeletedTour.mockImplementation(placeId => { + const mode = plannerActions.mode as { placeId?: number } + if (mode.placeId === placeId) plannerActions.mode = { type: 'neutral' } + }) + const view = renderPage({ + activeTab: 'tour-planner', + enabledAddons: { packing: true, budget: true, documents: true, collab: true, tours: true }, + toursEnabled: true, + tours: [savedTour], + handleDeleteTour, + setDeletePlaceId, + confirmDeletePlace, + }) + + fireEvent.click(await screen.findByRole('button', { name: 'Delete Ridge walk' })) + expect(handleDeleteTour).toHaveBeenCalledWith(savedTour.place_id) + view.rerender() + let confirmation = [...confirmDialogs].reverse().find(dialog => dialog.isOpen) + expect(confirmation).toMatchObject({ title: 'common.delete', confirmLabel: 'tours.delete.confirmAction' }) + expect(screen.getByRole('heading', { name: 'Edit tour' })).toBeInTheDocument() + + const onClose = confirmation?.onClose as (() => void) | undefined + onClose?.() + expect(setDeletePlaceId).toHaveBeenCalledWith(null) + expect(confirmDeletePlace).not.toHaveBeenCalled() + expect(screen.getByRole('heading', { name: 'Edit tour' })).toBeInTheDocument() + + fireEvent.click(screen.getByRole('button', { name: 'Delete Ridge walk' })) + view.rerender() + confirmation = [...confirmDialogs].reverse().find(dialog => dialog.isOpen) + const onConfirm = confirmation?.onConfirm as (() => void | Promise) | undefined + await act(async () => { await onConfirm?.() }) + expect(confirmDeletePlace).toHaveBeenCalledOnce() + expect(plannerActions.forgetDeletedTour).toHaveBeenCalledWith(savedTour.place_id) + view.rerender() + expect(screen.getByText('Plan a tour')).toBeInTheDocument() + + confirmDeletePlace.mockResolvedValue(null) + plannerActions.forgetDeletedTour.mockClear() + plannerActions.mode = { type: 'edit-saved', placeId: savedTour.place_id } + fireEvent.click(screen.getByRole('button', { name: 'Delete Ridge walk' })) + view.rerender() + confirmation = [...confirmDialogs].reverse().find(dialog => dialog.isOpen) + const failedConfirm = confirmation?.onConfirm as (() => void | Promise) | undefined + await act(async () => { await failedConfirm?.() }) + expect(plannerActions.forgetDeletedTour).not.toHaveBeenCalled() + expect(screen.getByRole('option', { name: /Ridge walk/ })).toBeInTheDocument() + view.rerender() + expect(screen.getByRole('heading', { name: 'Edit tour' })).toBeInTheDocument() + }) + + it('excludes Tours from the mobile TRIP-PLAN Places pool when the addon is enabled', () => { + const tourPlace = buildPlace({ id: 92, name: 'Mobile ridge', tour_place_id: 92 }) + const tourPlaceIds = new Set([tourPlace.id]) + renderPage({ + isMobile: true, + mobileSidebarOpen: 'right', + toursEnabled: true, + tourPlaceIds, + places: [tourPlace], + }) + + expect(props('places').toursEnabled).toBe(true) + expect(props('places').excludePlaceIds).toBe(tourPlaceIds) + }) + + it('passes Tour facet identity to desktop and mobile TRIP-PLAN day cards', () => { + const tourPlaceIds = new Set([92]) + const view = renderPage({ activeTab: 'plan', toursEnabled: true, tourPlaceIds }) + + expect(props('dayPlan').tourPlaceIds).toBe(tourPlaceIds) + + hookState = { ...hookState, isMobile: true, mobileSidebarOpen: 'left' } + view.rerender() + expect(props('dayPlan').tourPlaceIds).toBe(tourPlaceIds) + }) + it('FE-PAGE-TPW-062: opens GPX geometry in the read-only Planner path without mounting the general detail dialog', async () => { const gpxPlace = buildPlace({ id: 91, name: 'Imported ridge GPX', lat: 47.1, lng: 11.2, route_geometry: '[[47.1,11.2,500],[47.2,11.3,550]]' }) const gpxTour = { diff --git a/client/src/pages/tripPlanner/useTripPlanner.test.tsx b/client/src/pages/tripPlanner/useTripPlanner.test.tsx index 55e510c89..13ba9abe8 100644 --- a/client/src/pages/tripPlanner/useTripPlanner.test.tsx +++ b/client/src/pages/tripPlanner/useTripPlanner.test.tsx @@ -1319,7 +1319,7 @@ describe('useTripPlanner — place CRUD', () => { expect(actions.assignPlaceToDay).toHaveBeenCalledWith(42, 7, 900, 2) }) - it('permanently deletes a Tour without Place restore and forgets only its stale assignment undo', async () => { + it('preserves legacy deletion behavior for a dormant Tour and forgets only its stale assignment undo', async () => { const tourPlace = buildPlace({ id: 1, name: 'Ridge walk', tour_place_id: 1 }) const unrelatedUndo = vi.fn() const tourAssignmentUndo = vi.fn() @@ -1345,6 +1345,48 @@ describe('useTripPlanner — place CRUD', () => { expect(tourAssignmentUndo).not.toHaveBeenCalled() }) + it('allows permanent Tour deletion only in TOUR-PLANNER and leaves cancellation inert', async () => { + const tourPlace = buildPlace({ id: 1, name: 'Ridge walk', tour_place_id: 1 }) + seedTrip({ places: [tourPlace] }) + useAddonStore.setState({ addons: [{ id: 'tours', enabled: true } as never], loaded: true }) + vi.spyOn(toursApi, 'list').mockResolvedValue({ tours: [] }) + actions.deletePlace.mockResolvedValue({ success: true, tourPlaceIds: [1] }) + + const { result } = await renderPlanner() + await waitFor(() => expect(result.current.toursEnabled).toBe(true)) + + act(() => { + result.current.handleDeletePlace(1) + }) + expect(result.current.deletePlaceId).toBeNull() + expect(actions.deletePlace).not.toHaveBeenCalled() + + act(() => { + result.current.handleDeleteTour(1) + }) + expect(result.current.deletePlaceId).toBe(1) + act(() => { + result.current.setDeletePlaceId(null) + }) + await act(async () => { + await result.current.confirmDeletePlace() + }) + expect(actions.deletePlace).not.toHaveBeenCalled() + + act(() => { + result.current.handleDeleteTour(1) + }) + let deletedTourId: number | null = null + await act(async () => { + deletedTourId = await result.current.confirmDeletePlace() + }) + expect(actions.deletePlace).toHaveBeenCalledWith(42, 1) + expect(actions.deletePlace).toHaveBeenCalledTimes(1) + expect(deletedTourId).toBe(1) + expect(actions.addPlace).not.toHaveBeenCalled() + expect(result.current.canUndo).toBe(false) + }) + it('uses the server facet result to suppress Undo for a dormant Tour when Tours is off', async () => { const dormantPlace = buildPlace({ id: 1, name: 'Dormant route' }) seedTrip({ places: [dormantPlace] }) @@ -1385,14 +1427,39 @@ describe('useTripPlanner — place CRUD', () => { }) it('FE-TP-HOOK-057: a failing delete surfaces the server message', async () => { - seedTrip({ places: [buildPlace({ id: 1, lat: 1, lng: 2 })] }) - actions.deletePlace.mockRejectedValue(new Error('place is locked')) + const place = buildPlace({ id: 1, lat: 1, lng: 2, tour_place_id: 1 }) + const listedTour: TourListItem = { + place_id: 1, + name: 'Ridge walk', + tour_type: 'hike', + distance: 4, + elevation_gain: 100, + elevation_loss: 80, + duration: null, + difficulty: null, + wanderer_ref: null, + match_confidence: 1, + tour_group_id: null, + max_hiking_difficulty: 2, + planned: false, + caution: false, + } + seedTrip({ places: [place] }) + useAddonStore.setState({ addons: [{ id: 'tours', enabled: true } as never], loaded: true }) + vi.spyOn(toursApi, 'list').mockResolvedValue({ tours: [listedTour] }) + actions.deletePlace.mockRejectedValue(new Error('Tour is locked')) const { result } = await renderPlanner() - act(() => { result.current.handleDeletePlace(1) }) - await act(async () => { await result.current.confirmDeletePlace() }) + await waitFor(() => expect(result.current.tours).toEqual([listedTour])) + act(() => { result.current.handleDeleteTour(1) }) + let failedTourId: number | null = 1 + await act(async () => { failedTourId = await result.current.confirmDeletePlace() }) - expect(toasts.some(t => t.message === 'place is locked' && t.type === 'error')).toBe(true) + expect(failedTourId).toBeNull() + expect(result.current.tours).toEqual([listedTour]) + expect(useTripStore.getState().places).toContain(place) + expect(toasts.some(t => t.message === 'Tour is locked' && t.type === 'error')).toBe(true) + expect(toasts.some(t => t.type === 'success')).toBe(false) }) it('FE-TP-HOOK-058: a bulk delete restores every place with its assignments on undo', async () => { @@ -1751,10 +1818,13 @@ describe('useTripPlanner — day plan CRUD', () => { }) it('FE-TP-HOOK-066: removing a Tour assignment can be undone without deleting the Tour', async () => { - const place = buildPlace({ id: 1, lat: 1, lng: 2, tour_place_id: 1 }) + const place = buildPlace({ id: 1, lat: 1, lng: 2, tour_place_id: 1, route_geometry: '[[1,2],[3,4]]' }) seedTrip({ places: [place], - assignments: { '7': [buildAssignment({ id: 10, day_id: 7, place, order_index: 4 })] }, + assignments: { + '7': [buildAssignment({ id: 10, day_id: 7, place, order_index: 4 })], + '8': [buildAssignment({ id: 11, day_id: 8, place, order_index: 2 })], + }, }) const { result } = await renderPlanner() @@ -1763,6 +1833,9 @@ describe('useTripPlanner — day plan CRUD', () => { expect(actions.removeAssignment).toHaveBeenCalledWith(42, 7, 10) expect(actions.deletePlace).not.toHaveBeenCalled() expect(actions.addPlace).not.toHaveBeenCalled() + expect(useTripStore.getState().places).toContainEqual(expect.objectContaining({ id: place.id, route_geometry: place.route_geometry })) + expect(useTripStore.getState().assignments['8']).toHaveLength(1) + expect(useTripStore.getState().assignments['8'][0].id).toBe(11) await act(async () => { await result.current.undo() }) expect(actions.assignPlaceToDay).toHaveBeenCalledWith(42, 7, 1, 4) diff --git a/client/src/pages/tripPlanner/useTripPlanner.ts b/client/src/pages/tripPlanner/useTripPlanner.ts index 4dfd4e831..e0e58d3f5 100644 --- a/client/src/pages/tripPlanner/useTripPlanner.ts +++ b/client/src/pages/tripPlanner/useTripPlanner.ts @@ -2415,13 +2415,25 @@ export function useTripPlanner() { setStayDraft(draft) }, []) - const handleDeletePlace = useCallback((placeId) => { - if (!can('place_edit', trip)) return - setDeletePlaceId(placeId) - }, [can, trip]) + 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 + 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]) => @@ -2432,7 +2444,8 @@ export function useTripPlanner() { const deletedTour = Array.isArray(deletion?.tourPlaceIds) ? deletion.tourPlaceIds.includes(deletePlaceId) : isTourPlace(deletePlaceId) - void reloadTourPlaceIds() + if (deletedTour) invalidateTourPlaceIds({ removedPlaceIds: [deletePlaceId] }) + else void reloadTourPlaceIds() if (selectedPlaceId === deletePlaceId) setSelectedPlaceId(null) updateRouteForDay(selectedDayId) toast.success(t('trip.toast.placeDeleted')) @@ -2457,8 +2470,12 @@ export function useTripPlanner() { } }) } - } catch (err: unknown) { toast.error(err instanceof Error ? err.message : t('common.unknownError')) } - }, [deletePlaceId, tripId, toast, selectedPlaceId, selectedDayId, updateRouteForDay, pushUndo, forgetPlace, reloadTourPlaceIds, isTourPlace]) + 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 @@ -3072,6 +3089,7 @@ export function useTripPlanner() { route, routeWalking, routeSegments, routeInfo, setRoute, setRouteInfo, updateRouteForDay, handleSelectDay, handlePlaceClick, handleMarkerClick, handleMapClick, handleMapContextMenu, openAddPlaceFromPoi, handlePoiClick, handleSavePlace, openPlaceEditor, handleDeletePlace, confirmDeletePlace, confirmDeletePlaces, confirmChangeCategory, + handleDeleteTour, handleAssignToDay, handleMoveToDay, handleRemoveAssignment, handleReorder, handleReorderDays, handleAddDay, dayAdd, handleUpdateDayTitle, ...dayDelete, ...dayClear, diff --git a/client/src/store/slices/placesSlice.test.ts b/client/src/store/slices/placesSlice.test.ts index 6ab305dea..93a8a8369 100644 --- a/client/src/store/slices/placesSlice.test.ts +++ b/client/src/store/slices/placesSlice.test.ts @@ -133,6 +133,30 @@ describe('placesSlice', () => { }); describe('deletePlace', () => { + it('removes a deleted Tour and all local assignments after server success', async () => { + const tour = buildPlace({ id: 10, trip_id: 1, tour_place_id: 10, route_geometry: '[[1,2],[3,4]]' }) + const ordinary = buildPlace({ id: 20, trip_id: 1 }) + const tourDayOne = buildAssignment({ id: 100, day_id: 1, place: tour }) + const ordinaryDayOne = buildAssignment({ id: 101, day_id: 1, place: ordinary }) + const tourDayTwo = buildAssignment({ id: 200, day_id: 2, place: tour }) + seedStore(useTripStore, { + places: [tour, ordinary], + assignments: { '1': [tourDayOne, ordinaryDayOne], '2': [tourDayTwo] }, + }) + server.use( + http.delete('/api/trips/1/places/10', () => + HttpResponse.json({ success: true, tourPlaceIds: [10] }), + ), + ) + + const result = await useTripStore.getState().deletePlace(1, tour.id) + + expect(result).toMatchObject({ success: true, tourPlaceIds: [tour.id] }) + expect(useTripStore.getState().places).toEqual([ordinary]) + expect(useTripStore.getState().assignments['1']).toEqual([ordinaryDayOne]) + expect(useTripStore.getState().assignments['2']).toEqual([]) + }) + it('FE-TSLICE-PLACE-007: rethrows the server message and keeps the pool intact', async () => { const place = buildPlace({ id: 10, trip_id: 1 }); seedStore(useTripStore, { places: [place] }); diff --git a/client/tests/unit/mobile/trip/MBrowseActionsSheet.tours.test.tsx b/client/tests/unit/mobile/trip/MBrowseActionsSheet.tours.test.tsx index 7c20fea12..62e2fe57e 100644 --- a/client/tests/unit/mobile/trip/MBrowseActionsSheet.tours.test.tsx +++ b/client/tests/unit/mobile/trip/MBrowseActionsSheet.tours.test.tsx @@ -61,6 +61,8 @@ describe('MBrowseActionsSheet Tour assignment picker', () => { const planner = mobilePlanner({ handleAssignToDay }); const { rerender, shell } = renderDayPicker(planner); + expect(screen.queryByRole('button', { name: 'Delete' })).not.toBeInTheDocument(); + const dayOne = screen.getByRole('button', { name: /Day one/ }); const dayTwo = screen.getByRole('button', { name: /Day two/ }); expect(dayOne).toBeDisabled(); diff --git a/client/tests/unit/slices/assignmentsSlice.test.ts b/client/tests/unit/slices/assignmentsSlice.test.ts index 4a3c62cca..eed11a9f5 100644 --- a/client/tests/unit/slices/assignmentsSlice.test.ts +++ b/client/tests/unit/slices/assignmentsSlice.test.ts @@ -247,15 +247,19 @@ describe('assignmentsSlice', () => { }); it('FE-ASSIGN-004b: removeAssignment success removes from store', async () => { - const place = buildPlace({ id: 10, trip_id: 1 }); + const place = buildPlace({ id: 10, trip_id: 1, tour_place_id: 10, route_geometry: '[[1,2],[3,4]]' }); const assignment = buildAssignment({ id: 100, day_id: 1, place }); + const otherDayAssignment = buildAssignment({ id: 101, day_id: 2, place }); seedStore(useTripStore, { - assignments: { '1': [assignment] }, + places: [place], + assignments: { '1': [assignment], '2': [otherDayAssignment] }, }); await useTripStore.getState().removeAssignment(1, 1, 100); expect(useTripStore.getState().assignments['1']).toHaveLength(0); + expect(useTripStore.getState().places).toContainEqual(expect.objectContaining({ id: place.id, route_geometry: place.route_geometry })); + expect(useTripStore.getState().assignments['2']).toEqual([otherDayAssignment]); }); });