From 51ab30f4362049a9760c5515b66bf2c9c3e4a4c6 Mon Sep 17 00:00:00 2001 From: "Julien G." <66769052+jubnl@users.noreply.github.com> Date: Fri, 1 May 2026 01:43:19 +0200 Subject: [PATCH] Bug fixes - April 30th 2026 (#936) * fix: hotel day-range clamping in ReservationModal + stale assignment_id on accommodation clear (issues #929, #934) * ReservationModal hotel start/end pickers now use findIndex-based positional clamping instead of raw ID arithmetic, matching the fix applied to DayDetailPanel in 8e05ba7. Prevents inverted start_day_id/end_day_id on trips with non-monotonic day IDs. * Clearing accommodation_id on a hotel reservation now forces assignment_id to null in the save payload, removing the stale day-assignment link that had no UI path to clear. * Migration: swaps inverted start_day_id/end_day_id pairs in day_accommodations where start.day_number > end.day_number, recovering existing corrupt rows from the pre-fix picker bug. * Tests FE-PLANNER-RESMODAL-050/051/052 cover both fixes. * fix: preserve line breaks and wrap long URLs in notes fields (#930) Add remark-breaks to all reservation/place notes markdown renderers so single newlines render as
, and add wordBreak/overflowWrap styles so long unbroken URLs (e.g. booking.com tracking links) wrap correctly. * fix: delete linked budget item when accommodation or reservation is deleted (#933) Deleting an accommodation or reservation now removes any budget item linked via reservation_id, preventing orphan entries in the Budget page. Also fixes a pre-existing payload-shape bug where budget:deleted was broadcast with {id} instead of {itemId}, breaking live updates for collaborators when a reservation price was cleared. Tests added: ACCOM-006, RESV-009b, BUDGET-004b. * fix: restore scroll position in mobile Plan and Places sidebars on reopen (issue #932) Both DayPlanSidebar and PlacesSidebar have their own internal scroll containers (overflowY: auto). Scroll events don't bubble, so previous attempts that tracked scrollTop on the outer portal div never fired. Each sidebar now accepts initialScrollTop and onScrollTopChange props. The internal scroll container saves its scrollTop via onScrollTopChange on every scroll event, and restores it via useLayoutEffect on mount (before the browser paints, so no visible flash). TripPlannerPage holds the saved values in refs (mobilePlanScrollTopRef, mobilePlacesScrollTopRef) and passes them through on each portal mount. * fix(map): prevent auto zoom-out when opening/closing place inspector (issue #921) Both Leaflet and Mapbox GL renderers now gate fitBounds strictly on fitKey increments from the parent. Selecting or dismissing a place inspector changes paddingOpts (via hasInspector) but no longer triggers a re-fit that zoomed the map out to the full trip extent when no day was selected. Also removes the zoom-12 visibility gate on Leaflet route info pills so they render at all zoom levels when a route is active. * fix: translate mobile bottom-nav tab labels (issue #931) Replaced hardcoded English labels in BottomNav with t() lookups using the same translation keys as the desktop navbar (nav.myTrips, admin.addons.catalog.*.name). --- .../src/components/Layout/BottomNav.test.tsx | 41 ++++- client/src/components/Layout/BottomNav.tsx | 24 ++- client/src/components/Map/MapView.test.tsx | 50 +++++- client/src/components/Map/MapView.tsx | 15 +- client/src/components/Map/MapViewGL.test.tsx | 164 ++++++++++++++++++ client/src/components/Map/MapViewGL.tsx | 11 +- .../src/components/Planner/DayPlanSidebar.tsx | 17 +- .../src/components/Planner/PlaceInspector.tsx | 7 +- .../src/components/Planner/PlacesSidebar.tsx | 13 +- .../Planner/ReservationModal.test.tsx | 101 ++++++++++- .../components/Planner/ReservationModal.tsx | 16 +- .../components/Planner/ReservationsPanel.tsx | 7 +- client/src/index.css | 2 +- client/src/pages/TripPlannerPage.test.tsx | 50 ++++++ client/src/pages/TripPlannerPage.tsx | 6 +- server/src/db/migrations.ts | 11 ++ server/src/routes/days.ts | 5 +- server/src/routes/reservations.ts | 7 +- server/src/services/dayService.ts | 13 +- server/src/services/reservationService.ts | 11 +- server/tests/integration/budget.test.ts | 19 ++ server/tests/integration/days.test.ts | 42 +++++ server/tests/integration/reservations.test.ts | 37 ++++ 23 files changed, 599 insertions(+), 70 deletions(-) create mode 100644 client/src/components/Map/MapViewGL.test.tsx diff --git a/client/src/components/Layout/BottomNav.test.tsx b/client/src/components/Layout/BottomNav.test.tsx index d603a6a9..9244efc9 100644 --- a/client/src/components/Layout/BottomNav.test.tsx +++ b/client/src/components/Layout/BottomNav.test.tsx @@ -19,8 +19,10 @@ vi.mock('react-router-dom', async () => { import { render, screen, fireEvent } from '../../../tests/helpers/render'; import userEvent from '@testing-library/user-event'; import { useAuthStore } from '../../store/authStore'; +import { useSettingsStore } from '../../store/settingsStore'; +import { useAddonStore } from '../../store/addonStore'; import { resetAllStores, seedStore } from '../../../tests/helpers/store'; -import { buildUser } from '../../../tests/helpers/factories'; +import { buildUser, buildSettings } from '../../../tests/helpers/factories'; import BottomNav from './BottomNav'; const currentUser = buildUser({ id: 1, username: 'testuser', email: 'test@example.com' }); @@ -39,7 +41,7 @@ describe('BottomNav', () => { it('FE-COMP-BOTTOMNAV-002: shows Trips nav link', () => { render(); - expect(screen.getByText('Trips')).toBeInTheDocument(); + expect(screen.getByText('My Trips')).toBeInTheDocument(); }); it('FE-COMP-BOTTOMNAV-003: shows Profile button', () => { @@ -99,4 +101,39 @@ describe('BottomNav', () => { // Sheet should be closed — username no longer visible (only the nav Profile text remains) expect(screen.queryByText('testuser')).not.toBeInTheDocument(); }); + + it('FE-COMP-BOTTOMNAV-010: Trips label translates when language is fr', () => { + seedStore(useSettingsStore, { settings: buildSettings({ language: 'fr' }) }); + render(); + expect(screen.getByText('Mes voyages')).toBeInTheDocument(); + }); + + it('FE-COMP-BOTTOMNAV-011: Profile label translates when language is fr', () => { + seedStore(useSettingsStore, { settings: buildSettings({ language: 'fr' }) }); + render(); + expect(screen.getByText('Profil')).toBeInTheDocument(); + }); + + it('FE-COMP-BOTTOMNAV-012: addon labels translate when language is fr', () => { + seedStore(useSettingsStore, { settings: buildSettings({ language: 'fr' }) }); + seedStore(useAddonStore, { + addons: [ + { id: 'vacay', name: 'Vacay', type: 'global', icon: 'calendar', enabled: true }, + { id: 'atlas', name: 'Atlas', type: 'global', icon: 'globe', enabled: true }, + { id: 'journey', name: 'Journey', type: 'global', icon: 'compass', enabled: true }, + ], + }); + render(); + expect(screen.getByText('Vacances')).toBeInTheDocument(); + expect(screen.getByText('Atlas')).toBeInTheDocument(); + expect(screen.getByText('Journal de voyage')).toBeInTheDocument(); + }); + + it('FE-COMP-BOTTOMNAV-013: unknown addon id is not rendered', () => { + seedStore(useAddonStore, { + addons: [{ id: 'foo', name: 'Foo Addon', type: 'global', icon: 'star', enabled: true }], + }); + render(); + expect(screen.queryByText('Foo Addon')).not.toBeInTheDocument(); + }); }); diff --git a/client/src/components/Layout/BottomNav.tsx b/client/src/components/Layout/BottomNav.tsx index 16aa9ffd..59fd9854 100644 --- a/client/src/components/Layout/BottomNav.tsx +++ b/client/src/components/Layout/BottomNav.tsx @@ -7,14 +7,10 @@ import { useTranslation } from '../../i18n' import { Plane, CalendarDays, Globe, Compass, User, Settings, Shield, LogOut, X } from 'lucide-react' import type { LucideIcon } from 'lucide-react' -const BASE_ITEMS: { to: string; label: string; icon: LucideIcon; addonId?: string }[] = [ - { to: '/trips', label: 'Trips', icon: Plane }, -] - -const ADDON_NAV: Record = { - vacay: { to: '/vacay', label: 'Vacay', icon: CalendarDays }, - atlas: { to: '/atlas', label: 'Atlas', icon: Globe }, - journey: { to: '/journey', label: 'Journey', icon: Compass }, +const ADDON_NAV: Record = { + vacay: { icon: CalendarDays, labelKey: 'admin.addons.catalog.vacay.name' }, + atlas: { icon: Globe, labelKey: 'admin.addons.catalog.atlas.name' }, + journey: { icon: Compass, labelKey: 'admin.addons.catalog.journey.name' }, } export default function BottomNav() { @@ -25,11 +21,13 @@ export default function BottomNav() { const globalAddons = addons.filter(a => a.type === 'global' && a.enabled) const [showProfile, setShowProfile] = useState(false) - const items = [...BASE_ITEMS] - for (const addon of globalAddons) { - const nav = ADDON_NAV[addon.id] - if (nav) items.push(nav) - } + const items: { to: string; label: string; icon: LucideIcon }[] = [ + { to: '/trips', label: t('nav.myTrips'), icon: Plane }, + ...globalAddons.flatMap(addon => { + const nav = ADDON_NAV[addon.id] + return nav ? [{ to: `/${addon.id}`, label: t(nav.labelKey), icon: nav.icon }] : [] + }), + ] return ( <> diff --git a/client/src/components/Map/MapView.test.tsx b/client/src/components/Map/MapView.test.tsx index 7a6a4c16..902a8f32 100644 --- a/client/src/components/Map/MapView.test.tsx +++ b/client/src/components/Map/MapView.test.tsx @@ -7,6 +7,16 @@ import { resetAllStores } from '../../../tests/helpers/store' import { buildPlace } from '../../../tests/helpers/factories' import * as photoService from '../../services/photoService' +const mapMock = vi.hoisted(() => ({ + panTo: vi.fn(), + setView: vi.fn(), + fitBounds: vi.fn(), + getZoom: vi.fn().mockReturnValue(10), + on: vi.fn(), + off: vi.fn(), + panBy: vi.fn(), +})) + vi.mock('react-leaflet', () => ({ MapContainer: ({ children }: any) =>
{children}
, TileLayer: () =>
, @@ -27,15 +37,7 @@ vi.mock('react-leaflet', () => ({ Polyline: ({ positions }: any) =>
, CircleMarker: () =>
, Circle: () =>
, - useMap: () => ({ - panTo: vi.fn(), - setView: vi.fn(), - fitBounds: vi.fn(), - getZoom: () => 10, - on: vi.fn(), - off: vi.fn(), - panBy: vi.fn(), - }), + useMap: () => mapMock, useMapEvents: () => ({}), })) @@ -79,6 +81,7 @@ function buildMapPlace(overrides: Record = {}) { } afterEach(() => { + vi.clearAllMocks() resetAllStores() }) @@ -216,4 +219,33 @@ describe('MapView', () => { render() expect(screen.getByTestId('marker')).toBeTruthy() }) + + it('FE-COMP-MAPVIEW-018: changing selectedPlaceId/hasInspector does not refit bounds (issue #921)', () => { + const places = [ + buildMapPlace({ id: 1, lat: 48.8584, lng: 2.2945 }), + buildMapPlace({ id: 2, lat: 48.86, lng: 2.337 }), + ] + const { rerender } = render() + const initialCount = mapMock.fitBounds.mock.calls.length + + // Toggle selectedPlaceId on — mimics opening place inspector (hasInspector flips, + // paddingOpts memo creates new object). fitBounds must NOT fire again. + rerender() + expect(mapMock.fitBounds).toHaveBeenCalledTimes(initialCount) + + // Toggle selectedPlaceId off — mimics closing inspector via X button. + rerender() + expect(mapMock.fitBounds).toHaveBeenCalledTimes(initialCount) + }) + + it('FE-COMP-MAPVIEW-019: bumping fitKey triggers a new fitBounds call', () => { + const places = [ + buildMapPlace({ id: 1, lat: 48.8584, lng: 2.2945 }), + ] + const { rerender } = render() + const afterFirst = mapMock.fitBounds.mock.calls.length + + rerender() + expect(mapMock.fitBounds.mock.calls.length).toBeGreaterThan(afterFirst) + }) }) diff --git a/client/src/components/Map/MapView.tsx b/client/src/components/Map/MapView.tsx index 5af4bea3..a4a9831d 100644 --- a/client/src/components/Map/MapView.tsx +++ b/client/src/components/Map/MapView.tsx @@ -186,7 +186,7 @@ function BoundsController({ places, fitKey, paddingOpts, hasDayDetail }: BoundsC } } } catch {} - }, [fitKey, places, paddingOpts, map, hasDayDetail]) + }, [fitKey]) // eslint-disable-line react-hooks/exhaustive-deps return null } @@ -233,18 +233,7 @@ interface RouteLabelProps { } function RouteLabel({ midpoint, walkingText, drivingText }: RouteLabelProps) { - const map = useMap() - const [visible, setVisible] = useState(map ? map.getZoom() >= 12 : false) - - useEffect(() => { - if (!map) return - const check = () => setVisible(map.getZoom() >= 12) - check() - map.on('zoomend', check) - return () => map.off('zoomend', check) - }, [map]) - - if (!visible || !midpoint) return null + if (!midpoint) return null const icon = L.divIcon({ className: 'route-info-pill', diff --git a/client/src/components/Map/MapViewGL.test.tsx b/client/src/components/Map/MapViewGL.test.tsx new file mode 100644 index 00000000..5a305a70 --- /dev/null +++ b/client/src/components/Map/MapViewGL.test.tsx @@ -0,0 +1,164 @@ +import React from 'react' +import { describe, it, expect, vi, afterEach, beforeEach } from 'vitest' +import { render } from '../../../tests/helpers/render' +import { act } from '@testing-library/react' +import { resetAllStores } from '../../../tests/helpers/store' +import { buildPlace } from '../../../tests/helpers/factories' +import { useSettingsStore } from '../../store/settingsStore' + +// Stable fake map so fitBounds call counts survive re-renders. +const glMap = vi.hoisted(() => ({ + on: vi.fn(), + off: vi.fn(), + once: vi.fn(), + loaded: vi.fn().mockReturnValue(true), + fitBounds: vi.fn(), + flyTo: vi.fn(), + jumpTo: vi.fn(), + getZoom: vi.fn().mockReturnValue(10), + addControl: vi.fn(), + removeControl: vi.fn(), + remove: vi.fn(), + addSource: vi.fn(), + getSource: vi.fn().mockReturnValue(null), + addLayer: vi.fn(), + setLayoutProperty: vi.fn(), + getStyle: vi.fn().mockReturnValue({ layers: [] }), + isStyleLoaded: vi.fn().mockReturnValue(true), + getCanvasContainer: vi.fn(() => document.createElement('div')), +})) + +vi.mock('mapbox-gl', () => ({ + default: { + accessToken: '', + Map: vi.fn(() => glMap), + Marker: vi.fn(() => ({ + setLngLat: vi.fn().mockReturnThis(), + addTo: vi.fn().mockReturnThis(), + remove: vi.fn(), + getElement: vi.fn(() => document.createElement('div')), + })), + LngLatBounds: vi.fn(() => ({ extend: vi.fn().mockReturnThis() })), + NavigationControl: vi.fn(), + }, +})) +vi.mock('mapbox-gl/dist/mapbox-gl.css', () => ({})) + +vi.mock('./mapboxSetup', () => ({ + isStandardFamily: vi.fn(() => false), + supportsCustom3d: vi.fn(() => false), + wantsTerrain: vi.fn(() => false), + addCustom3dBuildings: vi.fn(), + addTerrainAndSky: vi.fn(), +})) + +vi.mock('./locationMarkerMapbox', () => ({ + attachLocationMarker: vi.fn(() => ({ update: vi.fn() })), +})) + +vi.mock('./reservationsMapbox', () => ({ + ReservationMapboxOverlay: vi.fn().mockImplementation(() => ({ update: vi.fn() })), +})) + +vi.mock('../../hooks/useGeolocation', () => ({ + useGeolocation: vi.fn(() => ({ + position: null, + mode: 'off', + error: null, + cycleMode: vi.fn(), + setMode: vi.fn(), + })), +})) + +vi.mock('../../services/photoService', () => ({ + getCached: vi.fn(() => null), + isLoading: vi.fn(() => false), + fetchPhoto: vi.fn(), + onThumbReady: vi.fn(() => () => {}), + getAllThumbs: vi.fn(() => ({})), +})) + +import { MapViewGL } from './MapViewGL' + +function buildMapPlace(overrides: Record = {}) { + return { + ...buildPlace(), + category_name: null, + category_color: null, + category_icon: null, + ...overrides, + } as any +} + +beforeEach(() => { + useSettingsStore.setState({ + settings: { + ...useSettingsStore.getState().settings, + map_provider: 'mapbox-gl', + mapbox_access_token: 'pk.test_token', + mapbox_style: 'mapbox://styles/mapbox/streets-v12', + mapbox_3d_enabled: false, + }, + } as any) +}) + +afterEach(() => { + vi.clearAllMocks() + resetAllStores() +}) + +describe('MapViewGL', () => { + it('FE-COMP-MAPVIEWGL-001: opening place inspector does not refit bounds (issue #921)', async () => { + const places = [ + buildMapPlace({ id: 1, lat: 48.8584, lng: 2.2945 }), + buildMapPlace({ id: 2, lat: 48.86, lng: 2.337 }), + ] + + const { rerender } = render( + , + ) + await act(async () => {}) + const after_initial = glMap.fitBounds.mock.calls.length + + // Selecting a place flips hasInspector → paddingOpts memo changes. + // fitBounds must NOT fire again (this was the bug). + rerender( + , + ) + await act(async () => {}) + expect(glMap.fitBounds).toHaveBeenCalledTimes(after_initial) + }) + + it('FE-COMP-MAPVIEWGL-002: closing inspector does not refit bounds (issue #921)', async () => { + const places = [ + buildMapPlace({ id: 1, lat: 48.8584, lng: 2.2945 }), + ] + + const { rerender } = render( + , + ) + await act(async () => {}) + const after_initial = glMap.fitBounds.mock.calls.length + + // Closing inspector (X button) clears selectedPlaceId → hasInspector=false → new paddingOpts. + rerender( + , + ) + await act(async () => {}) + expect(glMap.fitBounds).toHaveBeenCalledTimes(after_initial) + }) + + it('FE-COMP-MAPVIEWGL-003: bumping fitKey triggers a new fitBounds call', async () => { + const places = [ + buildMapPlace({ id: 1, lat: 48.8584, lng: 2.2945 }), + ] + + const { rerender } = render() + await act(async () => {}) + const after_first = glMap.fitBounds.mock.calls.length + + rerender() + await act(async () => {}) + expect(glMap.fitBounds.mock.calls.length).toBeGreaterThan(after_first) + }) +}) diff --git a/client/src/components/Map/MapViewGL.tsx b/client/src/components/Map/MapViewGL.tsx index 557e2647..9be41e17 100644 --- a/client/src/components/Map/MapViewGL.tsx +++ b/client/src/components/Map/MapViewGL.tsx @@ -507,13 +507,10 @@ export function MapViewGL({ return { top, right: rightWidth + 40, bottom, left: leftWidth + 40 } }, [leftWidth, rightWidth, hasInspector, hasDayDetail]) - // Also fit when the places collection changes so the initial render - // zooms to the trip instead of the default center. - const placeBoundsKey = useMemo( - () => places.filter(p => p.lat && p.lng).map(p => `${p.id}:${p.lat}:${p.lng}`).join('|'), - [places] - ) + const prevFitKey = useRef(-1) useEffect(() => { + if (fitKey === prevFitKey.current) return + prevFitKey.current = fitKey const map = mapRef.current if (!map) return const target = dayPlaces.length > 0 ? dayPlaces : places @@ -533,7 +530,7 @@ export function MapViewGL({ } if (map.loaded()) run() else map.once('load', run) - }, [fitKey, placeBoundsKey, paddingOpts, mapbox3d]) // eslint-disable-line react-hooks/exhaustive-deps + }, [fitKey]) // eslint-disable-line react-hooks/exhaustive-deps // flyTo selected place useEffect(() => { diff --git a/client/src/components/Planner/DayPlanSidebar.tsx b/client/src/components/Planner/DayPlanSidebar.tsx index 967cbfcc..14fcd498 100644 --- a/client/src/components/Planner/DayPlanSidebar.tsx +++ b/client/src/components/Planner/DayPlanSidebar.tsx @@ -2,7 +2,7 @@ interface DragDataPayload { placeId?: string; assignmentId?: string; noteId?: string; reservationId?: string; fromDayId?: string; phase?: 'single' | 'start' | 'middle' | 'end' } declare global { interface Window { __dragData: DragDataPayload | null } } -import React, { useState, useEffect, useRef, useMemo } from 'react' +import React, { useState, useEffect, useLayoutEffect, useRef, useMemo } from 'react' import ReactDOM from 'react-dom' import { ChevronDown, ChevronRight, ChevronUp, ChevronsDownUp, ChevronsUpDown, Navigation, RotateCcw, ExternalLink, Clock, Pencil, GripVertical, Ticket, Plus, FileText, Check, Trash2, Info, MapPin, Star, Heart, Camera, Lightbulb, Flag, Bookmark, Train, Bus, Plane, Car, Ship, Coffee, ShoppingBag, AlertTriangle, FileDown, Lock, Hotel, Utensils, Users, Undo2, X, Route as RouteIcon } from 'lucide-react' @@ -14,6 +14,7 @@ import PlaceAvatar from '../shared/PlaceAvatar' import { useContextMenu, ContextMenu } from '../shared/ContextMenu' import Markdown from 'react-markdown' import remarkGfm from 'remark-gfm' +import remarkBreaks from 'remark-breaks' import WeatherWidget from '../Weather/WeatherWidget' import { useToast } from '../shared/Toast' import { getCategoryIcon } from '../shared/categoryIcons' @@ -190,6 +191,8 @@ interface DayPlanSidebarProps { onEditTransport?: (reservation: Reservation) => void onEditReservation?: (reservation: Reservation) => void onAddBookingToAssignment?: (dayId: number, assignmentId: number) => void + initialScrollTop?: number + onScrollTopChange?: (top: number) => void } const DayPlanSidebar = React.memo(function DayPlanSidebar({ @@ -218,6 +221,8 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar({ onEditTransport, onEditReservation, onAddBookingToAssignment, + initialScrollTop, + onScrollTopChange, }: DayPlanSidebarProps) { const toast = useToast() const { t, language, locale } = useTranslation() @@ -270,6 +275,12 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar({ } | null>(null) const inputRef = useRef(null) const dragDataRef = useRef(null) + const scrollContainerRef = useRef(null) + useLayoutEffect(() => { + if (scrollContainerRef.current && initialScrollTop) { + scrollContainerRef.current.scrollTop = initialScrollTop + } + }, []) const initedTransportIds = useRef(new Set()) // Speichert Drag-Daten als Backup (dataTransfer geht bei Re-Render verloren) // Remember which assignment we last auto-scrolled into view so we don't // keep yanking the user back whenever they scroll away while the same @@ -1117,7 +1128,7 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar({
{/* Tagesliste */} -
+
onScrollTopChange?.((e.currentTarget as HTMLElement).scrollTop)}> {days.map((day, index) => { const isSelected = selectedDayId === day.id const isExpanded = expandedDays.has(day.id) @@ -2228,7 +2239,7 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar({ {res.notes && (
{t('reservations.notes')}
-
{res.notes}
+
{res.notes}
)} diff --git a/client/src/components/Planner/PlaceInspector.tsx b/client/src/components/Planner/PlaceInspector.tsx index be1da19b..9e09f1ff 100644 --- a/client/src/components/Planner/PlaceInspector.tsx +++ b/client/src/components/Planner/PlaceInspector.tsx @@ -2,6 +2,7 @@ import React, { useState, useEffect, useRef, useCallback, useMemo } from 'react' import { openFile } from '../../utils/fileDownload' import Markdown from 'react-markdown' import remarkGfm from 'remark-gfm' +import remarkBreaks from 'remark-breaks' import { X, Clock, MapPin, ExternalLink, Phone, Euro, Edit2, Trash2, Plus, Minus, ChevronDown, ChevronUp, FileText, Upload, File, FileImage, Star, Navigation, Users, Mountain, TrendingUp } from 'lucide-react' import PlaceAvatar from '../shared/PlaceAvatar' import { mapsApi } from '../../api/client' @@ -349,8 +350,8 @@ export default function PlaceInspector({ {/* Notes */} {place.notes && ( -
- {place.notes} +
+ {place.notes}
)} @@ -399,7 +400,7 @@ export default function PlaceInspector({
)}
- {res.notes &&
{res.notes}
} + {res.notes &&
{res.notes}
} {(() => { const meta = typeof res.metadata === 'string' ? JSON.parse(res.metadata || '{}') : (res.metadata || {}) if (!meta || Object.keys(meta).length === 0) return null diff --git a/client/src/components/Planner/PlacesSidebar.tsx b/client/src/components/Planner/PlacesSidebar.tsx index a3d45d63..50303768 100644 --- a/client/src/components/Planner/PlacesSidebar.tsx +++ b/client/src/components/Planner/PlacesSidebar.tsx @@ -1,6 +1,6 @@ import React from 'react' import ReactDOM from 'react-dom' -import { useState, useMemo, useEffect, useRef, useCallback } from 'react' +import { useState, useMemo, useEffect, useLayoutEffect, useRef, useCallback } from 'react' import { Search, Plus, X, CalendarDays, Pencil, Trash2, ExternalLink, Navigation, Upload, ChevronDown, Check, MapPin, Eye, Route } from 'lucide-react' import PlaceAvatar from '../shared/PlaceAvatar' import { getCategoryIcon } from '../shared/categoryIcons' @@ -34,6 +34,8 @@ interface PlacesSidebarProps { onCategoryFilterChange?: (categoryIds: Set) => void onPlacesFilterChange?: (filter: string) => void pushUndo?: (label: string, undoFn: () => Promise | void) => void + initialScrollTop?: number + onScrollTopChange?: (top: number) => void } interface MemoPlaceRowProps { @@ -145,6 +147,7 @@ const MemoPlaceRow = React.memo(function MemoPlaceRow({ const PlacesSidebar = React.memo(function PlacesSidebar({ tripId, places, categories, assignments, selectedDayId, selectedPlaceId, onPlaceClick, onAddPlace, onAssignToDay, onEditPlace, onDeletePlace, onBulkDeletePlaces, onBulkDeleteConfirm, days, isMobile, onCategoryFilterChange, onPlacesFilterChange, pushUndo, + initialScrollTop, onScrollTopChange, }: PlacesSidebarProps) { const { t } = useTranslation() const toast = useToast() @@ -159,6 +162,12 @@ const PlacesSidebar = React.memo(function PlacesSidebar({ const [sidebarDropFile, setSidebarDropFile] = useState(null) const [sidebarDragOver, setSidebarDragOver] = useState(false) const sidebarDragCounter = useRef(0) + const scrollContainerRef = useRef(null) + useLayoutEffect(() => { + if (scrollContainerRef.current && initialScrollTop) { + scrollContainerRef.current.scrollTop = initialScrollTop + } + }, []) const handleSidebarDragEnter = (e: React.DragEvent) => { if (!canEditPlaces) return @@ -636,7 +645,7 @@ const PlacesSidebar = React.memo(function PlacesSidebar({ )} {/* Liste */} -
+
onScrollTopChange?.((e.currentTarget as HTMLElement).scrollTop)}> {filtered.length === 0 ? (
diff --git a/client/src/components/Planner/ReservationModal.test.tsx b/client/src/components/Planner/ReservationModal.test.tsx index 4cf9c208..537782bf 100644 --- a/client/src/components/Planner/ReservationModal.test.tsx +++ b/client/src/components/Planner/ReservationModal.test.tsx @@ -1,4 +1,4 @@ -// FE-PLANNER-RESMODAL-001 to FE-PLANNER-RESMODAL-035 +// FE-PLANNER-RESMODAL-001 to FE-PLANNER-RESMODAL-052 import { render, screen, waitFor, fireEvent } from '../../../tests/helpers/render'; import userEvent from '@testing-library/user-event'; import { http, HttpResponse } from 'msw'; @@ -723,4 +723,103 @@ describe('ReservationModal', () => { expect.objectContaining({ type: 'hotel' }) ); }); + + // ── Hotel day-range picker — non-monotonic IDs (issue #929) ─────────────── + // Mirrors DayDetailPanel-056/057 for the ReservationModal path. + // ID layout: day_number 1-9 → IDs 17-25, day_number 10-16 → IDs 1-7. + + function buildNonMonotonicDaysRM() { + return [ + buildDay({ id: 17, trip_id: 1, date: '2026-04-30', day_number: 1 }), + buildDay({ id: 18, trip_id: 1, date: '2026-05-01', day_number: 2 }), + buildDay({ id: 19, trip_id: 1, date: '2026-05-02', day_number: 3 }), + buildDay({ id: 20, trip_id: 1, date: '2026-05-03', day_number: 4 }), + buildDay({ id: 21, trip_id: 1, date: '2026-05-04', day_number: 5 }), + buildDay({ id: 22, trip_id: 1, date: '2026-05-05', day_number: 6 }), + buildDay({ id: 23, trip_id: 1, date: '2026-05-06', day_number: 7 }), + buildDay({ id: 24, trip_id: 1, date: '2026-05-07', day_number: 8 }), + buildDay({ id: 25, trip_id: 1, date: '2026-05-08', day_number: 9 }), + buildDay({ id: 1, trip_id: 1, date: '2026-05-09', day_number: 10 }), + buildDay({ id: 2, trip_id: 1, date: '2026-05-10', day_number: 11 }), + buildDay({ id: 3, trip_id: 1, date: '2026-05-11', day_number: 12 }), + buildDay({ id: 4, trip_id: 1, date: '2026-05-12', day_number: 13 }), + buildDay({ id: 5, trip_id: 1, date: '2026-05-13', day_number: 14 }), + buildDay({ id: 6, trip_id: 1, date: '2026-05-14', day_number: 15 }), + buildDay({ id: 7, trip_id: 1, date: '2026-05-15', day_number: 16 }), + ] as any[]; + } + + it('FE-PLANNER-RESMODAL-050: non-monotonic IDs — end picker with low ID does not clobber start', async () => { + const onSave = vi.fn().mockResolvedValue(undefined); + const days = buildNonMonotonicDaysRM(); + + render(); + + // Switch to hotel type + await userEvent.click(screen.getByRole('button', { name: /^Accommodation$/i })); + await userEvent.type(screen.getByPlaceholderText(/e\.g\. Lufthansa/i), 'Overlap Hotel'); + + // Open start picker (first "Select day" trigger) and select Day 1 (id=17) + const startTrigger = () => screen.getAllByRole('button').filter(b => b.textContent?.includes('Select day') || b.textContent?.startsWith('Day '))[0]; + await userEvent.click(startTrigger()); + await userEvent.click(screen.getAllByRole('button').find(b => b.textContent?.startsWith('Day 1') && !b.textContent?.startsWith('Day 1 ') || b.textContent?.trim() === 'Day 1')!); + + // Open end picker and select Day 16 (id=7, low ID but last positionally) + const endTrigger = () => screen.getAllByRole('button').filter(b => b.textContent?.includes('Select day') || /^Day \d+/.test(b.textContent?.trim() ?? ''))[1]; + await userEvent.click(endTrigger()); + await userEvent.click(screen.getAllByRole('button').find(b => b.textContent?.startsWith('Day 16'))!); + + await userEvent.click(screen.getByRole('button', { name: /^Add$/i })); + + await waitFor(() => expect(onSave).toHaveBeenCalled()); + const saved = onSave.mock.calls[0][0]; + // start must stay id=17 (Day 1) — old Math.max would clobber it to id=7 + expect(saved.create_accommodation?.start_day_id).toBe(17); + expect(saved.create_accommodation?.end_day_id).toBe(7); + }); + + it('FE-PLANNER-RESMODAL-051: non-monotonic IDs — start picker does not collapse end when start has high ID', async () => { + const onSave = vi.fn().mockResolvedValue(undefined); + const days = buildNonMonotonicDaysRM(); + + render(); + + await userEvent.click(screen.getByRole('button', { name: /^Accommodation$/i })); + await userEvent.type(screen.getByPlaceholderText(/e\.g\. Lufthansa/i), 'Span Hotel'); + + // Set end to Day 16 (id=7) first + const endTrigger = () => screen.getAllByRole('button').filter(b => b.textContent?.includes('Select day') || /^Day \d+/.test(b.textContent?.trim() ?? ''))[1]; + await userEvent.click(endTrigger()); + await userEvent.click(screen.getAllByRole('button').find(b => b.textContent?.startsWith('Day 16'))!); + + // Set start to Day 9 (id=25, high ID but earlier by position than Day 16) + // Old code: Math.max(25, 7) = 25 → end collapses to Day 9. + // New code: position(id=25)=8 < position(id=7)=15 → end stays id=7. + const startTrigger = () => screen.getAllByRole('button').filter(b => b.textContent?.includes('Select day') || /^Day \d+/.test(b.textContent?.trim() ?? ''))[0]; + await userEvent.click(startTrigger()); + await userEvent.click(screen.getAllByRole('button').find(b => b.textContent?.startsWith('Day 9'))!); + + await userEvent.click(screen.getByRole('button', { name: /^Add$/i })); + + await waitFor(() => expect(onSave).toHaveBeenCalled()); + const saved = onSave.mock.calls[0][0]; + expect(saved.create_accommodation?.start_day_id).toBe(25); // Day 9 + expect(saved.create_accommodation?.end_day_id).toBe(7); // Day 16 — must NOT have collapsed + }); + + it('FE-PLANNER-RESMODAL-052: hotel with no accommodation_id sends assignment_id as null (issue #934)', async () => { + const onSave = vi.fn().mockResolvedValue(undefined); + // Hotel reservation with assignment_id set but no accommodation + const res = buildReservation({ + id: 10, title: 'Stale Hotel', type: 'hotel', status: 'confirmed', + accommodation_id: null, assignment_id: 99, + } as any); + + render(); + + await userEvent.click(screen.getByRole('button', { name: /^Update$/i })); + + await waitFor(() => expect(onSave).toHaveBeenCalled()); + expect(onSave.mock.calls[0][0].assignment_id).toBeNull(); + }); }); diff --git a/client/src/components/Planner/ReservationModal.tsx b/client/src/components/Planner/ReservationModal.tsx index f5ec6f13..c50b388f 100644 --- a/client/src/components/Planner/ReservationModal.tsx +++ b/client/src/components/Planner/ReservationModal.tsx @@ -196,7 +196,7 @@ export function ReservationModal({ isOpen, onClose, onSave, reservation, days, p reservation_end_time: form.type === 'hotel' ? null : (combinedEndTime || null), location: form.location, confirmation_number: form.confirmation_number, notes: form.notes, - assignment_id: form.assignment_id || null, + assignment_id: (form.type === 'hotel' && !form.accommodation_id) ? null : (form.assignment_id || null), accommodation_id: form.type === 'hotel' ? (form.accommodation_id || null) : null, metadata: Object.keys(metadata).length > 0 ? metadata : null, endpoints: [], @@ -459,7 +459,12 @@ export function ReservationModal({ isOpen, onClose, onSave, reservation, days, p set('hotel_start_day', value)} + onChange={value => setForm(prev => ({ + ...prev, + hotel_start_day: value, + hotel_end_day: days.findIndex(d => d.id === value) > days.findIndex(d => d.id === prev.hotel_end_day) + ? value : prev.hotel_end_day, + }))} placeholder={t('reservations.meta.selectDay')} options={days.map(d => { const dateBadge = d.date ? (formatDate(d.date, locale) ?? undefined) : undefined @@ -477,7 +482,12 @@ export function ReservationModal({ isOpen, onClose, onSave, reservation, days, p set('hotel_end_day', value)} + onChange={value => setForm(prev => ({ + ...prev, + hotel_start_day: days.findIndex(d => d.id === value) < days.findIndex(d => d.id === prev.hotel_start_day) + ? value : prev.hotel_start_day, + hotel_end_day: value, + }))} placeholder={t('reservations.meta.selectDay')} options={days.map(d => { const dateBadge = d.date ? (formatDate(d.date, locale) ?? undefined) : undefined diff --git a/client/src/components/Planner/ReservationsPanel.tsx b/client/src/components/Planner/ReservationsPanel.tsx index 770d36a5..7dc1a686 100644 --- a/client/src/components/Planner/ReservationsPanel.tsx +++ b/client/src/components/Planner/ReservationsPanel.tsx @@ -11,6 +11,9 @@ import { ExternalLink, BookMarked, Lightbulb, Link2, Clock, ArrowRight, AlertCircle, } from 'lucide-react' import { openFile } from '../../utils/fileDownload' +import Markdown from 'react-markdown' +import remarkGfm from 'remark-gfm' +import remarkBreaks from 'remark-breaks' import type { Reservation, Day, TripFile, AssignmentsMap } from '../../types' interface AssignmentLookupEntry { @@ -364,7 +367,9 @@ function ReservationCard({ r, tripId, onEdit, onDelete, files = [], onNavigateTo {r.notes && (
{t('reservations.notes')}
-
{r.notes}
+
+ {r.notes} +
)} diff --git a/client/src/index.css b/client/src/index.css index 4332ffdc..01341c38 100644 --- a/client/src/index.css +++ b/client/src/index.css @@ -807,7 +807,7 @@ img[alt="TREK"] { .collab-note-md code, .collab-note-md-full code { font-size: 0.9em; padding: 1px 5px; border-radius: 4px; background: var(--bg-secondary); } .collab-note-md-full pre { padding: 10px 12px; border-radius: 8px; background: var(--bg-secondary); overflow-x: auto; margin: 0.5em 0; } .collab-note-md-full pre code { padding: 0; background: none; } -.collab-note-md a, .collab-note-md-full a { color: #3b82f6; text-decoration: underline; } +.collab-note-md a, .collab-note-md-full a { color: #3b82f6; text-decoration: underline; word-break: break-all; } .collab-note-md blockquote, .collab-note-md-full blockquote { border-left: 3px solid var(--border-primary); padding-left: 12px; margin: 0.5em 0; color: var(--text-muted); } .collab-note-md-full table { border-collapse: collapse; width: 100%; margin: 0.5em 0; } .collab-note-md-full th, .collab-note-md-full td { border: 1px solid var(--border-primary); padding: 4px 8px; font-size: 0.9em; } diff --git a/client/src/pages/TripPlannerPage.test.tsx b/client/src/pages/TripPlannerPage.test.tsx index 78096249..3ea43503 100644 --- a/client/src/pages/TripPlannerPage.test.tsx +++ b/client/src/pages/TripPlannerPage.test.tsx @@ -1474,6 +1474,56 @@ describe('TripPlannerPage', () => { }); }); + describe('FE-PAGE-PLANNER-051: Mobile Plan sidebar stays mounted after onPlaceClick (issue #932)', () => { + it('does not unmount the mobile Plan portal when a place is tapped, preserving scroll position', async () => { + vi.useFakeTimers(); + Object.defineProperty(window, 'innerWidth', { writable: true, configurable: true, value: 375 }); + + const place = buildPlace({ id: 1, trip_id: 42, lat: 48.8566, lng: 2.3522 }); + const assignment = buildAssignment({ id: 10, day_id: 99, place, order_index: 0 }); + seedTripStore({ id: 42 }); + seedStore(useTripStore, { + places: [place], + assignments: { '99': [assignment] }, + } as any); + + renderPlannerPage(42); + act(() => { vi.runAllTimers(); }); + vi.useRealTimers(); + + await waitFor(() => { + expect(screen.getByTestId('day-plan-sidebar')).toBeInTheDocument(); + }); + + // Open the mobile Plan portal via the bottom-nav Plan button (selector mirrors FE-PAGE-PLANNER-049). + const mobilePlanBtn = Array.from(document.body.querySelectorAll('button')).find( + b => b.textContent === 'Plan' && !b.getAttribute('title'), + ); + expect(mobilePlanBtn).toBeTruthy(); + await act(async () => { fireEvent.click(mobilePlanBtn!); }); + + await waitFor(() => { + expect(screen.getAllByTestId('day-plan-sidebar').length).toBe(2); + }); + + // The mock factory overwrites capturedDayPlanSidebarProps on each mount, + // so current holds the mobile portal instance's props. + const mobileOnPlaceClick = capturedDayPlanSidebarProps.current.onPlaceClick; + expect(typeof mobileOnPlaceClick).toBe('function'); + + await act(async () => { + mobileOnPlaceClick(place.id, assignment.id); + }); + + // Invariant: portal must NOT unmount — both instances persist. + // Pre-fix: collapses to 1 (setMobileSidebarOpen(null) destroyed scroll container). + // Post-fix: stays at 2, browser preserves scrollTop on the living DOM node. + expect(screen.getAllByTestId('day-plan-sidebar').length).toBe(2); + + Object.defineProperty(window, 'innerWidth', { writable: true, configurable: true, value: 1024 }); + }); + }); + describe('FE-PAGE-PLANNER-037: onExpandedDaysChange covers mapPlaces hidden logic', () => { it('calls onExpandedDaysChange to trigger mapPlaces hidden set computation', async () => { vi.useFakeTimers(); diff --git a/client/src/pages/TripPlannerPage.tsx b/client/src/pages/TripPlannerPage.tsx index 54cc4726..8be7cd58 100644 --- a/client/src/pages/TripPlannerPage.tsx +++ b/client/src/pages/TripPlannerPage.tsx @@ -272,6 +272,8 @@ export default function TripPlannerPage(): React.ReactElement | null { const [fitKey, setFitKey] = useState(0) const initialFitTripId = useRef(null) 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) @@ -1114,8 +1116,8 @@ export default function TripPlannerPage(): React.ReactElement | null {
{mobileSidebarOpen === 'left' - ? { handleSelectDay(id); setMobileSidebarOpen(null) }} onPlaceClick={(placeId, assignmentId) => { handlePlaceClick(placeId, assignmentId); setMobileSidebarOpen(null) }} onReorder={handleReorder} onUpdateDayTitle={handleUpdateDayTitle} onAssignToDay={handleAssignToDay} onRouteCalculated={(r) => { if (r) { setRoute(r.coordinates); setRouteInfo({ distance: r.distanceText, duration: r.durationText }) } }} reservations={reservations} visibleConnectionIds={visibleConnections} onToggleConnection={toggleConnection} onAddReservation={(dayId) => { setEditingReservation(null); tripActions.setSelectedDay(dayId); setShowReservationModal(true); setMobileSidebarOpen(null) }} onAddPlace={() => { setEditingPlace(null); setShowPlaceForm(true); setMobileSidebarOpen(null) }} onDayDetail={(day) => { setShowDayDetail(day); setSelectedPlaceId(null); selectAssignment(null); setMobileSidebarOpen(null) }} accommodations={tripAccommodations} 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} /> - : { handlePlaceClick(placeId); setMobileSidebarOpen(null) }} onAddPlace={() => { setEditingPlace(null); setShowPlaceForm(true); setMobileSidebarOpen(null) }} onAssignToDay={handleAssignToDay} onEditPlace={(place) => { setEditingPlace(place); setEditingAssignmentId(null); setShowPlaceForm(true); setMobileSidebarOpen(null) }} onDeletePlace={(placeId) => handleDeletePlace(placeId)} onBulkDeletePlaces={(ids) => setDeletePlaceIds(ids)} onBulkDeleteConfirm={(ids) => confirmDeletePlaces(ids)} days={days} isMobile onCategoryFilterChange={setMapCategoryFilter} onPlacesFilterChange={setMapPlacesFilter} pushUndo={pushUndo} /> + ? { handleSelectDay(id); setMobileSidebarOpen(null) }} onPlaceClick={(placeId, assignmentId) => { handlePlaceClick(placeId, assignmentId) }} onReorder={handleReorder} onUpdateDayTitle={handleUpdateDayTitle} onAssignToDay={handleAssignToDay} onRouteCalculated={(r) => { if (r) { setRoute(r.coordinates); setRouteInfo({ distance: r.distanceText, duration: r.durationText }) } }} reservations={reservations} visibleConnectionIds={visibleConnections} onToggleConnection={toggleConnection} onAddReservation={(dayId) => { setEditingReservation(null); tripActions.setSelectedDay(dayId); setShowReservationModal(true); setMobileSidebarOpen(null) }} onAddPlace={() => { setEditingPlace(null); setShowPlaceForm(true); setMobileSidebarOpen(null) }} onDayDetail={(day) => { setShowDayDetail(day); setSelectedPlaceId(null); selectAssignment(null); setMobileSidebarOpen(null) }} accommodations={tripAccommodations} 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 }} /> + : { handlePlaceClick(placeId); setMobileSidebarOpen(null) }} onAddPlace={() => { setEditingPlace(null); setShowPlaceForm(true); setMobileSidebarOpen(null) }} onAssignToDay={handleAssignToDay} onEditPlace={(place) => { setEditingPlace(place); setEditingAssignmentId(null); setShowPlaceForm(true); setMobileSidebarOpen(null) }} onDeletePlace={(placeId) => handleDeletePlace(placeId)} onBulkDeletePlaces={(ids) => setDeletePlaceIds(ids)} onBulkDeleteConfirm={(ids) => confirmDeletePlaces(ids)} days={days} isMobile onCategoryFilterChange={setMapCategoryFilter} onPlacesFilterChange={setMapPlacesFilter} pushUndo={pushUndo} initialScrollTop={mobilePlacesScrollTopRef.current} onScrollTopChange={(top) => { mobilePlacesScrollTopRef.current = top }} /> }
diff --git a/server/src/db/migrations.ts b/server/src/db/migrations.ts index 29640339..417a6b9c 100644 --- a/server/src/db/migrations.ts +++ b/server/src/db/migrations.ts @@ -2130,6 +2130,17 @@ function runMigrations(db: Database.Database): void { 'ON journey_entries(journey_id, entry_date, sort_order)' ); }, + // Swap inverted start_day_id/end_day_id pairs in day_accommodations caused + // by the old Math.min/Math.max picker bug (pre-8e05ba7) which used raw IDs + // instead of positional order on trips with non-monotonic day ID layouts. + () => { + db.exec(` + UPDATE day_accommodations + SET start_day_id = end_day_id, end_day_id = start_day_id + WHERE (SELECT day_number FROM days WHERE id = start_day_id) + > (SELECT day_number FROM days WHERE id = end_day_id) + `); + }, ]; if (currentVersion < migrations.length) { diff --git a/server/src/routes/days.ts b/server/src/routes/days.ts index ce967355..659b7096 100644 --- a/server/src/routes/days.ts +++ b/server/src/routes/days.ts @@ -117,10 +117,13 @@ accommodationsRouter.delete('/:id', authenticate, requireTripAccess, (req: Reque if (!dayService.getAccommodation(id, tripId)) return res.status(404).json({ error: 'Accommodation not found' }); - const { linkedReservationId } = dayService.deleteAccommodation(id); + const { linkedReservationId, deletedBudgetItemId } = dayService.deleteAccommodation(id); if (linkedReservationId) { broadcast(tripId, 'reservation:deleted', { reservationId: linkedReservationId }, req.headers['x-socket-id'] as string); } + if (deletedBudgetItemId) { + broadcast(tripId, 'budget:deleted', { itemId: deletedBudgetItemId }, req.headers['x-socket-id'] as string); + } res.json({ success: true }); broadcast(tripId, 'accommodation:deleted', { accommodationId: Number(id) }, req.headers['x-socket-id'] as string); diff --git a/server/src/routes/reservations.ts b/server/src/routes/reservations.ts index c5351caf..52cfdc63 100644 --- a/server/src/routes/reservations.ts +++ b/server/src/routes/reservations.ts @@ -129,7 +129,7 @@ router.put('/:id', authenticate, (req: Request, res: Response) => { const linked = db.prepare('SELECT id FROM budget_items WHERE trip_id = ? AND reservation_id = ?').get(tripId, id) as { id: number } | undefined; if (linked) { deleteBudgetItem(linked.id, tripId); - broadcast(tripId, 'budget:deleted', { id: linked.id }, req.headers['x-socket-id'] as string); + broadcast(tripId, 'budget:deleted', { itemId: linked.id }, req.headers['x-socket-id'] as string); } } @@ -179,12 +179,15 @@ router.delete('/:id', authenticate, (req: Request, res: Response) => { if (!checkPermission('reservation_edit', authReq.user.role, trip.user_id, authReq.user.id, trip.user_id !== authReq.user.id)) return res.status(403).json({ error: 'No permission' }); - const { deleted: reservation, accommodationDeleted } = deleteReservation(id, tripId); + const { deleted: reservation, accommodationDeleted, deletedBudgetItemId } = deleteReservation(id, tripId); if (!reservation) return res.status(404).json({ error: 'Reservation not found' }); if (accommodationDeleted) { broadcast(tripId, 'accommodation:deleted', { accommodationId: reservation.accommodation_id }, req.headers['x-socket-id'] as string); } + if (deletedBudgetItemId) { + broadcast(tripId, 'budget:deleted', { itemId: deletedBudgetItemId }, req.headers['x-socket-id'] as string); + } res.json({ success: true }); broadcast(tripId, 'reservation:deleted', { reservationId: Number(id) }, req.headers['x-socket-id'] as string); diff --git a/server/src/services/dayService.ts b/server/src/services/dayService.ts index 80b773ad..8b3b6361 100644 --- a/server/src/services/dayService.ts +++ b/server/src/services/dayService.ts @@ -292,14 +292,19 @@ export function updateAccommodation(id: string | number, existing: DayAccommodat return getAccommodationWithPlace(Number(id)); } -/** Delete accommodation and its linked reservation. Returns the linked reservation id if one existed. */ -export function deleteAccommodation(id: string | number): { linkedReservationId: number | null } { - // Delete linked reservation +/** Delete accommodation and its linked reservation (and any linked budget item). */ +export function deleteAccommodation(id: string | number): { linkedReservationId: number | null; deletedBudgetItemId: number | null } { const linkedRes = db.prepare('SELECT id FROM reservations WHERE accommodation_id = ?').get(Number(id)) as { id: number } | undefined; + let deletedBudgetItemId: number | null = null; if (linkedRes) { + const linkedBudget = db.prepare('SELECT id FROM budget_items WHERE reservation_id = ?').get(linkedRes.id) as { id: number } | undefined; + if (linkedBudget) { + db.prepare('DELETE FROM budget_items WHERE id = ?').run(linkedBudget.id); + deletedBudgetItemId = linkedBudget.id; + } db.prepare('DELETE FROM reservations WHERE id = ?').run(linkedRes.id); } db.prepare('DELETE FROM day_accommodations WHERE id = ?').run(id); - return { linkedReservationId: linkedRes ? linkedRes.id : null }; + return { linkedReservationId: linkedRes ? linkedRes.id : null, deletedBudgetItemId }; } diff --git a/server/src/services/reservationService.ts b/server/src/services/reservationService.ts index 354a14f7..f78200ca 100644 --- a/server/src/services/reservationService.ts +++ b/server/src/services/reservationService.ts @@ -418,9 +418,9 @@ export function updateReservation(id: string | number, tripId: string | number, return { reservation, accommodationChanged }; } -export function deleteReservation(id: string | number, tripId: string | number): { deleted: { id: number; title: string; type: string; accommodation_id: number | null } | undefined; accommodationDeleted: boolean } { +export function deleteReservation(id: string | number, tripId: string | number): { deleted: { id: number; title: string; type: string; accommodation_id: number | null } | undefined; accommodationDeleted: boolean; deletedBudgetItemId: number | null } { const reservation = db.prepare('SELECT id, title, type, accommodation_id FROM reservations WHERE id = ? AND trip_id = ?').get(id, tripId) as { id: number; title: string; type: string; accommodation_id: number | null } | undefined; - if (!reservation) return { deleted: undefined, accommodationDeleted: false }; + if (!reservation) return { deleted: undefined, accommodationDeleted: false, deletedBudgetItemId: null }; let accommodationDeleted = false; if (reservation.accommodation_id) { @@ -428,6 +428,11 @@ export function deleteReservation(id: string | number, tripId: string | number): accommodationDeleted = true; } + const linkedBudget = db.prepare('SELECT id FROM budget_items WHERE trip_id = ? AND reservation_id = ?').get(tripId, id) as { id: number } | undefined; + if (linkedBudget) { + db.prepare('DELETE FROM budget_items WHERE id = ?').run(linkedBudget.id); + } + db.prepare('DELETE FROM reservations WHERE id = ?').run(id); - return { deleted: reservation, accommodationDeleted }; + return { deleted: reservation, accommodationDeleted, deletedBudgetItemId: linkedBudget ? linkedBudget.id : null }; } diff --git a/server/tests/integration/budget.test.ts b/server/tests/integration/budget.test.ts index 53378f86..7a1854df 100644 --- a/server/tests/integration/budget.test.ts +++ b/server/tests/integration/budget.test.ts @@ -189,6 +189,25 @@ describe('Delete budget item', () => { .set('Cookie', authCookie(user.id)); expect(list.body.items).toHaveLength(0); }); + + it('BUDGET-004b — DELETE budget item does NOT delete its linked reservation', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const reservation = createReservation(testDb, trip.id, { title: 'Hotel Booking', type: 'hotel' }); + + const result = testDb.prepare( + 'INSERT INTO budget_items (trip_id, name, category, total_price, reservation_id) VALUES (?, ?, ?, ?, ?)' + ).run(trip.id, 'Hotel Cost', 'Accommodation', 250, reservation.id); + const itemId = result.lastInsertRowid as number; + + const del = await request(app) + .delete(`/api/trips/${trip.id}/budget/${itemId}`) + .set('Cookie', authCookie(user.id)); + expect(del.status).toBe(200); + + const reservationAfter = testDb.prepare('SELECT id FROM reservations WHERE id = ?').get(reservation.id); + expect(reservationAfter).toBeDefined(); + }); }); // ───────────────────────────────────────────────────────────────────────────── diff --git a/server/tests/integration/days.test.ts b/server/tests/integration/days.test.ts index 26c39f83..408b2d51 100644 --- a/server/tests/integration/days.test.ts +++ b/server/tests/integration/days.test.ts @@ -502,4 +502,46 @@ describe('Accommodations', () => { ).get(reservationBefore.id); expect(reservationAfter).toBeUndefined(); }); + + it('ACCOM-006 — DELETE accommodation also removes its linked budget item (issue #933)', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id, { title: 'Hotel Budget Trip' }); + const day1 = createDay(testDb, trip.id, { date: '2026-11-01' }); + const day2 = createDay(testDb, trip.id, { date: '2026-11-03' }); + const place = createPlace(testDb, trip.id, { name: 'Grand Hotel' }); + + // Create a hotel reservation that creates an accommodation and a linked budget item + const createRes = await request(app) + .post(`/api/trips/${trip.id}/reservations`) + .set('Cookie', authCookie(user.id)) + .send({ + title: 'Grand Hotel Stay', + type: 'hotel', + day_id: day1.id, + create_accommodation: { place_id: place.id, start_day_id: day1.id, end_day_id: day2.id }, + create_budget_entry: { total_price: 450, category: 'Accommodation' }, + }); + expect(createRes.status).toBe(201); + + const accommodationId = testDb.prepare( + 'SELECT id FROM day_accommodations WHERE trip_id = ?' + ).get(trip.id) as any; + expect(accommodationId).toBeDefined(); + + const budgetBefore = testDb.prepare( + 'SELECT id FROM budget_items WHERE trip_id = ?' + ).get(trip.id); + expect(budgetBefore).toBeDefined(); + + // Delete via the accommodation endpoint (the primary bug path) + const delRes = await request(app) + .delete(`/api/trips/${trip.id}/accommodations/${accommodationId.id}`) + .set('Cookie', authCookie(user.id)); + expect(delRes.status).toBe(200); + + const budgetAfter = testDb.prepare( + 'SELECT id FROM budget_items WHERE trip_id = ?' + ).get(trip.id); + expect(budgetAfter).toBeUndefined(); + }); }); diff --git a/server/tests/integration/reservations.test.ts b/server/tests/integration/reservations.test.ts index 63528671..9412871f 100644 --- a/server/tests/integration/reservations.test.ts +++ b/server/tests/integration/reservations.test.ts @@ -452,4 +452,41 @@ describe('Reservation accommodation delete', () => { ).get(accom.id); expect(accomAfter).toBeUndefined(); }); + + it('RESV-009b — DELETE reservation linked to accommodation also removes its linked budget item (issue #933)', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day1 = createDay(testDb, trip.id, { date: '2025-08-01' }); + const day2 = createDay(testDb, trip.id, { date: '2025-08-03' }); + const place = createPlace(testDb, trip.id, { name: 'Seaside Resort' }); + + const createRes = await request(app) + .post(`/api/trips/${trip.id}/reservations`) + .set('Cookie', authCookie(user.id)) + .send({ + title: 'Seaside Resort Stay', + type: 'hotel', + day_id: day1.id, + create_accommodation: { place_id: place.id, start_day_id: day1.id, end_day_id: day2.id }, + create_budget_entry: { total_price: 320, category: 'Accommodation' }, + }); + expect(createRes.status).toBe(201); + const reservationId = createRes.body.reservation.id; + + const budgetBefore = testDb.prepare( + 'SELECT id FROM budget_items WHERE trip_id = ? AND reservation_id = ?' + ).get(trip.id, reservationId); + expect(budgetBefore).toBeDefined(); + + // Delete via the reservation endpoint + const delRes = await request(app) + .delete(`/api/trips/${trip.id}/reservations/${reservationId}`) + .set('Cookie', authCookie(user.id)); + expect(delRes.status).toBe(200); + + const budgetAfter = testDb.prepare( + 'SELECT id FROM budget_items WHERE trip_id = ?' + ).get(trip.id); + expect(budgetAfter).toBeUndefined(); + }); });