mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-06-21 14:21:46 +00:00
fix: replace raw day-ID range checks with position-based helper (issue #889 follow-up)
Commit 8e05ba7 fixed the accommodation date-range pickers, but the
post-save state filters in DayDetailPanel and several other consumers
still compared `day.id >= start_day_id && day.id <= end_day_id`. With
non-monotonic ID layouts (day_number 1-9 → IDs 17-25, day_number 10-16
→ IDs 1-7) this made the just-saved accommodation immediately invisible
— matching the regression reported in the last comment of #889.
Introduces `isDayInAccommodationRange` in `client/src/utils/dayOrder.ts`
which compares positional order (`day_number` with `indexOf` fallback)
rather than raw IDs. Falls back to the old numeric comparison when
endpoint days are absent from the loaded array (sparse test data or
partial loads) so existing tests are unaffected.
Fixed call sites:
- DayDetailPanel.tsx (initial load, post-create, post-delete, post-edit-save)
- DayPlanSidebar.tsx (daily badge renderer)
- SharedTripPage.tsx (public share view)
- TripPDF.tsx (PDF export filter + sort)
Also declares `day_number?: number` on the client `Day` type (already
returned by the server but previously untyped).
Adds regression tests FE-PLANNER-DAYDETAIL-060/061/062 covering the
edit-save, create-save, and initial-load paths with the reporter's exact
non-monotonic ID layout.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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(<DayDetailPanel {...defaultProps} day={days[0]} days={days} />);
|
||||
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(<DayDetailPanel {...defaultProps} day={currentDay} days={days} places={[place]} />);
|
||||
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(<DayDetailPanel {...defaultProps} day={days[0]} days={days} />);
|
||||
await screen.findByText('Full Trip Hotel');
|
||||
|
||||
// Intermediate day (id=1, position 9): old filter: 1>=17 → false. New: 9 in [0,15] → visible.
|
||||
render(<DayDetailPanel {...defaultProps} day={days[9]} days={days} />);
|
||||
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 },
|
||||
|
||||
@@ -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?.()
|
||||
|
||||
@@ -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({
|
||||
</Tooltip>
|
||||
)}
|
||||
{(() => {
|
||||
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
|
||||
|
||||
@@ -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 })),
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
}
|
||||
Reference in New Issue
Block a user