fix(tours): restrict permanent deletion to tours workspace

This commit is contained in:
elmocito
2026-10-06 12:52:25 +02:00
parent 04c7491e2a
commit 8739b4c74d
17 changed files with 456 additions and 39 deletions
@@ -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(
<DayPlanSidebar
{...makeDefaultProps({
days: [day],
places: [place],
tourPlaceIds: new Set([place.id]),
assignments: { '10': [assignment] },
onPlaceClick,
onRemoveAssignment,
onDeletePlace,
})}
/>
)
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(<DayPlanSidebar {...makeDefaultProps({ days: [day], places: [place], tourPlaceIds: new Set([place.id]), assignments: { '10': [assignment] }, onDeletePlace })} />)
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 () => {
@@ -78,6 +78,7 @@ interface DayPlanSidebarProps {
trip: Trip
days: Day[]
places: Place[]
tourPlaceIds?: ReadonlySet<number>
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 (
@@ -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(<TourPlannerToursRail planner={planner()} tours={[tour]} {...railProps} canEdit onDeleteTour={onDeleteTour} />)
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(<TourPlannerToursRail planner={controller} tours={[tour]} {...railProps} />)
@@ -389,9 +389,10 @@ interface TourPlannerToursRailProps extends TourPermissionProps {
loading: boolean
onAssignToDay: (placeId: number, dayId: number) => void | boolean | Promise<void | boolean>
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 ? (
<TourDayMenu
days={days}
placeId={tour.place_id}
label={`${t('tours.addToDay')}: ${tour.name}`}
onPick={day => { if (canAssign) void onAssignToDay(tour.place_id, day.id) }}
/>
action={((canAssign && days.length > 0) || (canEdit && onDeleteTour)) ? (
<>
{canAssign && days.length > 0 && (
<TourDayMenu
days={days}
placeId={tour.place_id}
label={`${t('tours.addToDay')}: ${tour.name}`}
onPick={day => { if (canAssign) void onAssignToDay(tour.place_id, day.id) }}
/>
)}
{canEdit && onDeleteTour && (
<button
type="button"
aria-label={`${t('common.delete')} ${tour.name}`}
title={`${t('common.delete')}: ${tour.name}`}
disabled={isSaving}
onClick={event => { event.stopPropagation(); if (!isSaving) onDeleteTour(tour.place_id) }}
className="inline-flex h-8 w-8 items-center justify-center rounded-lg text-content-muted hover:text-danger focus-visible:outline focus-visible:outline-2 focus-visible:outline-accent disabled:opacity-50"
>
<Trash2 size={16} strokeWidth={1.9} aria-hidden="true" />
</button>
)}
</>
) : undefined}
/>
))}
@@ -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(<TourPlannerRail planner={result.current} />)
expect(screen.getByRole('heading', { name: 'Edit tour' })).toBeInTheDocument()
act(() => result.current.forgetDeletedTour(savedTour.place_id))
view.rerender(<TourPlannerRail planner={result.current} />)
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()
})
})
@@ -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<number>())
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,