diff --git a/client/src/components/PDF/TripPDF.tsx b/client/src/components/PDF/TripPDF.tsx index 1a5a3316..e5700b93 100644 --- a/client/src/components/PDF/TripPDF.tsx +++ b/client/src/components/PDF/TripPDF.tsx @@ -4,6 +4,7 @@ import { getCategoryIcon } from '../shared/categoryIcons' import { FileText, Info, Clock, MapPin, Navigation, Train, Plane, Bus, Car, Ship, Coffee, Ticket, Star, Heart, Camera, Flag, Lightbulb, AlertTriangle, ShoppingBag, Bookmark, Hotel, LogIn, LogOut, KeyRound, BedDouble, Utensils, Users, LucideIcon } from 'lucide-react' import { accommodationsApi, mapsApi } from '../../api/client' import type { Trip, Day, Place, Category, AssignmentsMap, DayNotesMap } from '../../types' +import { isDayInAccommodationRange, getDayOrder } from '../../utils/dayOrder' function renderLucideIcon(icon:LucideIcon, props = {}) { if (!_renderToStaticMarkup) return '' @@ -285,8 +286,12 @@ export async function downloadTripPDF({ trip, days, places, assignments, categor }).join('') const accommodationsForDay = (accommodations.accommodations || []).filter(a => - days.some(d => d.id >= a.start_day_id && d.id <= a.end_day_id && d.id === day?.id) - ).sort((a, b) => a.start_day_id - b.start_day_id) + day ? isDayInAccommodationRange(day, a.start_day_id, a.end_day_id, days) : false + ).sort((a, b) => { + const startA = days.find(d => d.id === a.start_day_id) + const startB = days.find(d => d.id === b.start_day_id) + return (startA ? getDayOrder(startA, days) : 0) - (startB ? getDayOrder(startB, days) : 0) + }) const accommodationDetails = accommodationsForDay.map(item => { const isCheckIn = day.id === item.start_day_id diff --git a/client/src/components/Planner/DayDetailPanel.test.tsx b/client/src/components/Planner/DayDetailPanel.test.tsx index 9bdc0256..a70fd9d5 100644 --- a/client/src/components/Planner/DayDetailPanel.test.tsx +++ b/client/src/components/Planner/DayDetailPanel.test.tsx @@ -1069,6 +1069,100 @@ describe('DayDetailPanel', () => { }); }); + // ── Post-save state filter — non-monotonic IDs (issue #889 follow-up) ──────── + + it('FE-PLANNER-DAYDETAIL-060: non-monotonic IDs — hotel stays visible after edit-save (issue #889 regression)', async () => { + const days = buildNonMonotonicDays(); + let getCallCount = 0; + server.use( + http.get('/api/trips/1/accommodations', () => { + getCallCount++; + const acc = getCallCount === 1 + // Initial load: single-day so old filter (17>=17 && 17<=17) passes — hotel visible, edit possible + ? { id: 1, place_id: 50, place_name: 'Span Hotel', place_address: null, start_day_id: 17, end_day_id: 17, check_in: null, check_out: null, confirmation: null } + // Post-save relist: full span — old filter (17>=17 && 17<=7) would drop it, new code keeps it + : { id: 1, place_id: 50, place_name: 'Span Hotel', place_address: null, start_day_id: 17, end_day_id: 7, check_in: null, check_out: null, confirmation: null }; + return HttpResponse.json({ accommodations: [acc] }); + }), + http.put('/api/trips/1/accommodations/1', async ({ request }) => { + const body = await request.json() as any; + return HttpResponse.json({ + accommodation: { id: 1, place_id: 50, place_name: 'Span Hotel', place_address: null, + start_day_id: body.start_day_id, end_day_id: body.end_day_id, + check_in: null, check_out: null, confirmation: null }, + }); + }), + ); + + render(); + await screen.findByText('Span Hotel'); + + // Pencil = 3rd button (index 2): collapse, close, pencil, remove + const allButtons = screen.getAllByRole('button'); + await userEvent.click(allButtons[2]); + + // Extend end picker to Day 16 (id=7) + await userEvent.click(getDayPickerTriggers()[1]); + await userEvent.click(screen.getAllByRole('button').find(b => b.textContent?.startsWith('Day 16'))!); + await userEvent.click(screen.getByRole('button', { name: /^Save$/i })); + + // Old code: 17>=17 && 17<=7 → false (hotel vanishes). New code: position 0 in [0,15] → visible. + await waitFor(() => { + expect(screen.getByText('Span Hotel')).toBeInTheDocument(); + }); + }); + + it('FE-PLANNER-DAYDETAIL-061: non-monotonic IDs — hotel appears after create-save on intermediate day', async () => { + const days = buildNonMonotonicDays(); + const place = buildPlace({ id: 55, name: 'Created Hotel' }); + // Current day: days[5] = id 22, position 5 (within any full-span range) + const currentDay = days[5]; + server.use( + http.post('/api/trips/1/accommodations', async ({ request }) => { + const body = await request.json() as any; + return HttpResponse.json({ + accommodation: { id: 200, place_id: 55, place_name: 'Created Hotel', place_address: null, + start_day_id: body.start_day_id, end_day_id: body.end_day_id, + check_in: null, check_out: null, confirmation: null }, + }); + }), + ); + + render(); + await userEvent.click(await screen.findByText(/Add accommodation/i)); + await userEvent.click(await screen.findByRole('button', { name: /Created Hotel/i })); + + // Extend end to Day 16 (id=7) — start stays at current day id=22 + await userEvent.click(getDayPickerTriggers()[1]); + await userEvent.click(screen.getAllByRole('button').find(b => b.textContent?.startsWith('Day 16'))!); + await userEvent.click(screen.getByRole('button', { name: /^Save$/i })); + + // Old code: 22>=22 && 22<=7 → false (hotel vanishes). New code: position 5 in [5,15] → visible. + await waitFor(() => { + expect(screen.getByText('Created Hotel')).toBeInTheDocument(); + }); + }); + + it('FE-PLANNER-DAYDETAIL-062: non-monotonic IDs — hotel shown on initial load when it spans the full trip', async () => { + const days = buildNonMonotonicDays(); + server.use( + http.get('/api/trips/1/accommodations', () => + HttpResponse.json({ + accommodations: [{ id: 1, place_id: 60, place_name: 'Full Trip Hotel', place_address: null, + start_day_id: 17, end_day_id: 7, check_in: null, check_out: null, confirmation: null }], + }) + ), + ); + + // Day 1 (id=17): old filter: 17>=17 && 17<=7 → false. New: position 0 in [0,15] → visible. + render(); + await screen.findByText('Full Trip Hotel'); + + // Intermediate day (id=1, position 9): old filter: 1>=17 → false. New: 9 in [0,15] → visible. + render(); + await screen.findByText('Full Trip Hotel'); + }); + it('FE-PLANNER-DAYDETAIL-040: 12h time format renders reservation time with AM/PM', async () => { seedStore(useSettingsStore, { settings: { time_format: '12h', temperature_unit: 'celsius', blur_booking_codes: false }, diff --git a/client/src/components/Planner/DayDetailPanel.tsx b/client/src/components/Planner/DayDetailPanel.tsx index 0d53f8ff..407db408 100644 --- a/client/src/components/Planner/DayDetailPanel.tsx +++ b/client/src/components/Planner/DayDetailPanel.tsx @@ -12,6 +12,7 @@ import CustomTimePicker from '../shared/CustomTimePicker' import { useSettingsStore } from '../../store/settingsStore' import { getLocaleForLanguage, useTranslation } from '../../i18n' import type { Day, Place, Category, Reservation, AssignmentsMap } from '../../types' +import { isDayInAccommodationRange } from '../../utils/dayOrder' const WEATHER_ICON_MAP = { Clear: Sun, Clouds: Cloud, Rain: CloudRain, Drizzle: CloudDrizzle, @@ -99,7 +100,7 @@ export default function DayDetailPanel({ day, days, places, categories = [], tri .then(data => { setAccommodations(data.accommodations || []) const allForDay = (data.accommodations || []).filter(a => - days.some(d => d.id >= a.start_day_id && d.id <= a.end_day_id && d.id === day?.id) + day ? isDayInAccommodationRange(day, a.start_day_id, a.end_day_id, days) : false ) setDayAccommodations(allForDay) setAccommodation(allForDay[0] || null) @@ -130,7 +131,7 @@ export default function DayDetailPanel({ day, days, places, categories = [], tri setAccommodations(updated) setAccommodation(newAcc) setDayAccommodations(updated.filter(a => - days.some(d => d.id >= a.start_day_id && d.id <= a.end_day_id && d.id === day?.id) + day ? isDayInAccommodationRange(day, a.start_day_id, a.end_day_id, days) : false )) setShowHotelPicker(false) setHotelForm({ check_in: '', check_in_end: '', check_out: '', confirmation: '', place_id: null }) @@ -154,7 +155,7 @@ export default function DayDetailPanel({ day, days, places, categories = [], tri const updated = accommodations.filter(a => a.id !== accommodation.id) setAccommodations(updated) setDayAccommodations(updated.filter(a => - days.some(d => d.id >= a.start_day_id && d.id <= a.end_day_id && d.id === day?.id) + day ? isDayInAccommodationRange(day, a.start_day_id, a.end_day_id, days) : false )) setAccommodation(null) onAccommodationChange?.() @@ -598,9 +599,9 @@ export default function DayDetailPanel({ day, days, places, categories = [], tri const all = d.accommodations || [] setAccommodations(all) setDayAccommodations(all.filter(a => - days.some(dd => dd.id >= a.start_day_id && dd.id <= a.end_day_id && dd.id === day?.id) + day ? isDayInAccommodationRange(day, a.start_day_id, a.end_day_id, days) : false )) - const acc = all.find(a => days.some(dd => dd.id >= a.start_day_id && dd.id <= a.end_day_id && dd.id === day?.id)) + const acc = all.find(a => day ? isDayInAccommodationRange(day, a.start_day_id, a.end_day_id, days) : false) setAccommodation(acc || null) }) onAccommodationChange?.() diff --git a/client/src/components/Planner/DayPlanSidebar.tsx b/client/src/components/Planner/DayPlanSidebar.tsx index b1eda2d3..7c44f091 100644 --- a/client/src/components/Planner/DayPlanSidebar.tsx +++ b/client/src/components/Planner/DayPlanSidebar.tsx @@ -21,6 +21,7 @@ import { useTripStore } from '../../store/tripStore' import { useCanDo } from '../../store/permissionsStore' import { useSettingsStore } from '../../store/settingsStore' import { useTranslation } from '../../i18n' +import { isDayInAccommodationRange } from '../../utils/dayOrder' import { formatDate, formatTime, dayTotalCost, currencyDecimals } from '../../utils/formatters' import { useDayNotes } from '../../hooks/useDayNotes' import Tooltip from '../shared/Tooltip' @@ -1214,7 +1215,7 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar({ )} {(() => { - const dayAccs = accommodations.filter(a => day.id >= a.start_day_id && day.id <= a.end_day_id) + const dayAccs = accommodations.filter(a => isDayInAccommodationRange(day, a.start_day_id, a.end_day_id, days)) // Sort: check-out first, then ongoing stays, then check-in last .sort((a, b) => { const aIsOut = a.end_day_id === day.id && a.start_day_id !== day.id diff --git a/client/src/pages/SharedTripPage.tsx b/client/src/pages/SharedTripPage.tsx index 09fa8669..e7c75c1f 100644 --- a/client/src/pages/SharedTripPage.tsx +++ b/client/src/pages/SharedTripPage.tsx @@ -10,6 +10,7 @@ import { getCategoryIcon } from '../components/shared/categoryIcons' import { createElement } from 'react' import { renderToStaticMarkup } from 'react-dom/server' import { Clock, MapPin, FileText, Train, Plane, Bus, Car, Ship, Ticket, Hotel, Map, Luggage, Wallet, MessageCircle } from 'lucide-react' +import { isDayInAccommodationRange } from '../utils/dayOrder' const TRANSPORT_TYPES = new Set(['flight', 'train', 'bus', 'car', 'cruise']) const TRANSPORT_ICONS = { flight: Plane, train: Train, bus: Bus, car: Car, cruise: Ship } @@ -184,7 +185,7 @@ export default function SharedTripPage() { const da = assignments[String(day.id)] || [] const notes = (dayNotes[String(day.id)] || []) const dayTransport = (reservations || []).filter((r: any) => TRANSPORT_TYPES.has(r.type) && r.reservation_time?.split('T')[0] === day.date) - const dayAccs = (accommodations || []).filter((a: any) => day.id >= a.start_day_id && day.id <= a.end_day_id) + const dayAccs = (accommodations || []).filter((a: any) => isDayInAccommodationRange(day, a.start_day_id, a.end_day_id, sortedDays)) const merged = [ ...da.map((a: any) => ({ type: 'place', k: a.order_index, data: a })), diff --git a/client/src/types.ts b/client/src/types.ts index 5701597a..8c9c3039 100644 --- a/client/src/types.ts +++ b/client/src/types.ts @@ -31,6 +31,7 @@ export interface Trip { export interface Day { id: number trip_id: number + day_number?: number date: string title: string | null notes: string | null diff --git a/client/src/utils/dayOrder.ts b/client/src/utils/dayOrder.ts new file mode 100644 index 00000000..8ef66046 --- /dev/null +++ b/client/src/utils/dayOrder.ts @@ -0,0 +1,23 @@ +import type { Day } from '../types' + +export const getDayOrder = (day: Day, days: Day[]): number => + day.day_number ?? days.indexOf(day) + +export const isDayInAccommodationRange = ( + day: Day, + startDayId: number, + endDayId: number, + days: Day[], +): boolean => { + const startDay = days.find(d => d.id === startDayId) + const endDay = days.find(d => d.id === endDayId) + if (!startDay || !endDay) { + // Endpoint days not in the loaded array (e.g. sparse test data or partial load). + // Fall back to numeric ID range — acceptable since non-monotonic IDs only arise when + // both endpoints are present in a fully-loaded trip's days list. + return day.id >= Math.min(startDayId, endDayId) && day.id <= Math.max(startDayId, endDayId) + } + const lo = Math.min(getDayOrder(startDay, days), getDayOrder(endDay, days)) + const hi = Math.max(getDayOrder(startDay, days), getDayOrder(endDay, days)) + return getDayOrder(day, days) >= lo && getDayOrder(day, days) <= hi +}