k1nq/TREK
1
0
Fork 0
mirror of https://github.com/mauriceboe/TREK.git synced 2026-10-18 21:16:47 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(pdf): print a day in the order the plan shows it

Browse Source 
The export ordered each day by itself, on day_plan_position — one global
number per booking, seeded automatically from the place count of whichever
day happened to render first. On any other day of a span it therefore pointed
at the wrong slot, and where it was still null the export fell back to the
order the API returned rows in, which is reservation_time ASC with nulls
first.

That is why the same trip printed correctly some of the time and swapped two
bookings the rest of it, and why the reporter's workaround worked: adding a
place and removing it again reseeded the position from the day they were
looking at.

The plan has never used that column. It picks transports per day, gives each
one the time that day shows, inserts them among the places and re-sorts the
whole day by the clock — and none of that was reachable from here, because
this file had its own copy of the ordering with subtly different rules.

So the copy is gone and the export goes through utils/dayMerge, the same
functions the plan uses. Two differences fall out of that, both of them the
plan's existing behaviour: a booking attached to a place is no longer printed
twice, and a multi-leg journey prints one row per leg at each leg's own time
rather than one row carrying every leg as a subtitle. The per-segment booking
code #1943 added is read back from the booking, since the leg object does not
carry it.

Closes #1978
This commit is contained in:
Maurice
2026-08-20 17:50:55 +02:00
committed by Maurice
parent f76ee87336
commit f574e75dbe
2 changed files with 179 additions and 51 deletions
Download Patch File Download Diff File Expand all files Collapse all files
client/src/components/PDF/TripPDF.test.ts
+112
View File
@@ -4,6 +4,7 @@ import { http, HttpResponse } from 'msw'
import { downloadTripPDF } from './TripPDF'
import { server } from '../../../tests/helpers/msw/server'
import { clearExchangeRateCache } from '../../hooks/useExchangeRates'
import { getMergedItems, getTransportForDay } from '../../utils/dayMerge'
// ── Helpers ───────────────────────────────────────────────────────────────────
@@ -1096,3 +1097,114 @@ describe('page breaks between days (#1292)', () => {
expect(html.slice(at - 10, at)).toContain('.pdf-flow ')
})
})
/**
* The order a day prints in (#1978).
*
* The export used to order a day by itself, on `day_plan_position` — one global
* number per booking, auto-seeded from whichever day happened to render first.
* On any other day of a span it therefore pointed at the wrong slot, and where
* it was still null the export fell back to the order the API returned rows in.
* That is why the same trip printed correctly some of the time and swapped two
* bookings the rest of it, and why adding a place and removing it again "fixed"
* it: the reseed happened to land on the day being looked at.
*
* It now goes through utils/dayMerge, the same functions the day plan uses, so
* the two cannot disagree. These cases are written from the plan's rules rather
* than from the old implementation's.
*/
describe('a printed day follows the plan (#1978)', () => {
const d1 = { id: 21, day_number: 1, title: 'Day One', date: '2025-06-01' } as any
const d2 = { id: 22, day_number: 2, title: 'Day Two', date: '2025-06-02' } as any
const srcdoc = () => getIframe()!.srcdoc
/** Where each needle first appears, so order can be asserted without parsing. */
const positions = (html: string, needles: string[]) => needles.map(n => html.indexOf(n))
it('orders the day by the clock, not by the order the API sent rows in', async () => {
await downloadTripPDF({
...minimalArgs,
trip: { id: 21, title: 'Order Trip', description: null, cover_image: null } as any,
days: [d1],
// As the API returns them: reservation_time ASC with nulls first.
reservations: [
{ id: 1, title: 'Untimed Transfer', type: 'transfer', day_id: 21, reservation_time: null },
{ id: 2, title: 'Evening Train', type: 'train', day_id: 21, reservation_time: '2025-06-01T19:30' },
{ id: 3, title: 'Morning Flight', type: 'flight', day_id: 21, reservation_time: '2025-06-01T07:15' },
],
})
const [morning, evening] = positions(srcdoc(), ['Morning Flight', 'Evening Train'])
expect(morning).toBeGreaterThan(-1)
expect(evening).toBeGreaterThan(-1)
expect(morning).toBeLessThan(evening)
})
/*
* The reported shape: a booking on the last day of a span whose global
* position was seeded from a busier earlier day, which sank it below
* everything on the day it actually belongs to.
*/
it('ignores a global position seeded from another day', async () => {
await downloadTripPDF({
...minimalArgs,
trip: { id: 22, title: 'Span Trip', description: null, cover_image: null } as any,
days: [d1, d2],
reservations: [
{
id: 4, title: 'Car Return', type: 'car', day_id: 21, end_day_id: 22,
reservation_time: '2025-06-01T10:00', reservation_end_time: '2025-06-02T09:00',
// Seeded from day one, where five places had already been placed.
day_plan_position: 5,
},
{ id: 5, title: 'Late Flight', type: 'flight', day_id: 22, reservation_time: '2025-06-02T18:00' },
],
})
const html = srcdoc()
// On day two the 09:00 return comes before the 18:00 flight, whatever the
// stale global position says.
const [ret, flight] = positions(html, ['Car Return', 'Late Flight'])
expect(ret).toBeGreaterThan(-1)
expect(flight).toBeGreaterThan(-1)
expect(ret).toBeLessThan(flight)
})
/*
* The strongest form of the same claim: whatever the plan would show, the
* print shows, in that order. Asserted against getMergedItems itself rather
* than against a hand-written expectation, so the two cannot drift apart
* again without this failing.
*/
it('prints a mixed day in exactly the order the plan merges it', async () => {
const reservations = [
{ id: 8, title: 'Untimed Transfer', type: 'transfer', day_id: 21, reservation_time: null },
{ id: 9, title: 'Evening Train', type: 'train', day_id: 21, reservation_time: '2025-06-01T19:30' },
{ id: 10, title: 'Morning Flight', type: 'flight', day_id: 21, reservation_time: '2025-06-01T07:15' },
]
const dayAssignments = [
{ id: 100, day_id: 21, order_index: 0, place: { id: 1, name: 'Museum', place_time: '11:00' } },
{ id: 101, day_id: 21, order_index: 1, place: { id: 2, name: 'Market', place_time: '16:00' } },
]
await downloadTripPDF({
...minimalArgs,
trip: { id: 24, title: 'Mixed Trip', description: null, cover_image: null } as any,
days: [d1],
assignments: { '21': dayAssignments } as any,
reservations,
})
const html = srcdoc()
const expected = getMergedItems({
dayAssignments,
dayNotes: [],
dayTransports: getTransportForDay({
reservations, dayId: 21, dayAssignmentIds: dayAssignments.map(a => a.id), days: [d1],
}),
dayId: 21,
}).map(item => (item.type === 'place' ? item.data.place.name : item.data.title))
const seen = expected.map(name => html.indexOf(name))
for (const at of seen) expect(at).toBeGreaterThan(-1)
expect(seen).toEqual([...seen].sort((a, b) => a - b))
})
})
client/src/components/PDF/TripPDF.tsx
+67 -51
View File
@@ -5,7 +5,7 @@ import { FileText, Info, Clock, MapPin, Navigation, Train, Plane, Bus, Car, Ship
import { accommodationsApi, mapsApi, pluginsApi } from '../../api/client'
import type { Trip, Day, Place, Category, AssignmentsMap, DayNote } from '../../types'
import { isDayInAccommodationRange, getDayOrder } from '../../utils/dayOrder'
import { hidesOnMiddleDay } from '../../utils/dayMerge'
import { hidesOnMiddleDay, getTransportForDay, getMergedItems, getSpanPhase, getDisplayTimeForDay } from '../../utils/dayMerge'
import { safeHexColor } from '../../utils/safeColor'
import { renderIconMarkup } from '../../utils/iconMarkup'
import { formatMoney, formatMoneySum, splitReservationDateTime, type MoneyEntry } from '../../utils/formatters'
@@ -209,22 +209,19 @@ export async function downloadTripPDF({ trip, days, places, assignments = {}, ca
const fxRates = needsFx ? await fetchExchangeRates(tripCur) : null
const totalCostLabel = formatMoneySum(allCostEntries, tripCur, loc || 'en', fxRates)
// Span helpers for multi-day transport (mirrors DayPlanSidebar logic)
const pdfGetDayOrder = (d: Day) => d.day_number
const pdfGetSpanPhase = (r: any, dayId: number): 'single' | 'start' | 'middle' | 'end' => {
const startId = r.day_id
const endId = r.end_day_id ?? startId
if (!startId || startId === endId) return 'single'
if (dayId === startId) return 'start'
if (dayId === endId) return 'end'
return 'middle'
}
const pdfGetDisplayTime = (r: any, dayId: number): string | null => {
const phase = pdfGetSpanPhase(r, dayId)
if (phase === 'end') return r.reservation_end_time || null
if (phase === 'middle') return null
return r.reservation_time || null
}
/*
* The span label is the only PDF-specific piece left here. Everything else
* about how a day is ordered — which transports belong to it, what time each
* one shows, where it sits among the places — comes from utils/dayMerge, the
* same functions the day plan itself uses (#1978).
*
* There used to be a second implementation in this file. It ordered by
* `day_plan_position`, which is one global number per booking, seeded from
* whichever day happened to render first; on any other day of a span it
* pointed at the wrong slot, and when it was still null the export fell back
* to the order the API returned. That is why the PDF was right some of the
* time and swapped two bookings the rest of it.
*/
const pdfGetSpanLabel = (r: any, phase: string): string | null => {
if (phase === 'single') return null
if (r.type === 'flight') return tr(`reservations.span.${phase === 'start' ? 'departure' : phase === 'end' ? 'arrival' : 'inTransit'}`)
@@ -234,47 +231,43 @@ export async function downloadTripPDF({ trip, days, places, assignments = {}, ca
if (r.type === 'parking') return tr(`reservations.span.${phase === 'start' ? 'dropOff' : phase === 'end' ? 'pickup' : 'ongoing'}`)
return tr(`reservations.span.${phase === 'start' ? 'start' : phase === 'end' ? 'end' : 'ongoing'}`)
}
const pdfGetTransportForDay = (dayId: number) => (reservations || []).filter(r => {
if (r.type === 'hotel') return false
const startId = r.day_id
const endId = r.end_day_id ?? startId
if (startId == null) return false
if (endId !== startId) {
const startDay = sorted.find(d => d.id === startId)
const endDay = sorted.find(d => d.id === endId)
const thisDay = sorted.find(d => d.id === dayId)
if (!startDay || !endDay || !thisDay) return false
return pdfGetDayOrder(thisDay) >= pdfGetDayOrder(startDay) && pdfGetDayOrder(thisDay) <= pdfGetDayOrder(endDay)
}
return startId === dayId
})
// Build day HTML
const daysHtml = sorted.map((day, di) => {
const assigned = assignments[String(day.id)] || []
const notes = (dayNotes || []).filter(n => n.day_id === day.id)
const assigned = (assignments[String(day.id)] || []).slice()
.sort((a: any, b: any) => (a.order_index ?? 0) - (b.order_index ?? 0))
const notes = (dayNotes || []).filter(n => n.day_id === day.id).slice()
.sort((a: any, b: any) => (a.sort_order ?? 0) - (b.sort_order ?? 0))
const cost = dayCost(assignments, day.id, loc, tripCur, fxRates)
// Reservations for this day (hotel rendered via accommodations block; car middle-phase rendered in sidebar header only,
// a multi-day parking's middle days nowhere at all)
const dayReservations = pdfGetTransportForDay(day.id)
.filter(r => !(r.type === 'car' && pdfGetSpanPhase(r, day.id) === 'middle'))
.filter(r => !hidesOnMiddleDay(r, day.id))
const merged = []
assigned.forEach(a => merged.push({ type: 'place', k: a.order_index ?? 0, data: a }))
notes.forEach(n => merged.push({ type: 'note', k: n.sort_order ?? 0, data: n }))
dayReservations.forEach(r => {
const pos = r.day_positions?.[day.id] ?? r.day_positions?.[String(day.id)] ?? r.day_plan_position ?? (merged.length > 0 ? Math.max(...merged.map(m => m.k)) + 0.5 : 0.5)
merged.push({ type: 'reservation', k: pos, data: r })
// Assembled exactly the way DayPlanSidebar assembles it, so the page and the
// print cannot disagree about what order a day is in.
const dayTransports = getTransportForDay({
reservations: reservations || [],
dayId: day.id,
dayAssignmentIds: assigned.map((a: any) => a.id),
days: sorted,
})
// Hotels come out in the accommodations block, a rental car's middle days
// appear only in the sidebar header, and a multi-day parking's middle days
// nowhere at all.
const merged = getMergedItems({
dayAssignments: assigned,
dayNotes: notes,
dayTransports,
dayId: day.id,
}).filter(item => {
if (item.type !== 'transport') return true
const r: any = item.data
if (r.type === 'car' && getSpanPhase(r, day.id) === 'middle') return false
return !hidesOnMiddleDay(r, day.id)
})
merged.sort((a, b) => a.k - b.k)
let pi = 0
const itemsHtml = merged.length === 0
? `<div class="empty-day">${escHtml(tr('dayplan.emptyDay'))}</div>`
: merged.map(item => {
if (item.type === 'reservation') {
if (item.type === 'transport') {
const r = item.data
const meta = typeof r.metadata === 'string' ? JSON.parse(r.metadata || '{}') : (r.metadata || {})
const icon = reservationIconSvg(r.type)
@@ -282,7 +275,30 @@ export async function downloadTripPDF({ trip, days, places, assignments = {}, ca
let subtitle = ''
// Flights render one subtitle line per leg (see below); everything else is a single line.
let subtitleLines: string[] = []
if (r.type === 'flight') {
/*
* A multi-leg booking arrives as one item per leg now, the way the
* day plan shows it, because the shared merge expands legs so each
* one can sit at its own time among the day's places. Previously
* the export printed a single row with every leg as a subtitle
* line, which put the second leg's departure next to the first
* leg's clock.
*/
if (r.__leg) {
const l = r.__leg
// The segment's own booking code, which the leg object does not
// carry — read back from the booking by index. At the gate that is
// the code the airline asks for (#1943), so losing it here would
// have been a real regression for a stopover flight.
const source = (r.type === 'train' ? getTrainLegs(r) : getFlightLegs(r))[l.index]
subtitleLines = [[
l.airline, l.flight_number, l.train_number,
l.platform ? `Gl. ${l.platform}` : '',
l.seat ? `Seat ${l.seat}` : '',
(l.from || l.to) ? [l.from, l.to].filter(Boolean).join(' → ') : '',
source?.confirmation_number,
].filter(Boolean).join(' · ')].filter(Boolean)
}
else if (r.type === 'flight') {
const legs = getFlightLegs(r)
if (legs.length > 1) {
// Multi-leg: one line per leg so every flight number + segment route is
@@ -324,9 +340,9 @@ export async function downloadTripPDF({ trip, days, places, assignments = {}, ca
else if (r.type === 'tour') subtitle = [meta.operator].filter(Boolean).join(' · ')
if (subtitleLines.length === 0 && subtitle) subtitleLines = [subtitle]
const locationLine = r.location || meta.location || ''
const phase = pdfGetSpanPhase(r, day.id)
const phase = getSpanPhase(r, day.id)
const spanLabel = pdfGetSpanLabel(r, phase)
const displayTime = pdfGetDisplayTime(r, day.id)
const displayTime = getDisplayTimeForDay(r, day.id)
// Start and end, the way the day plan draws it (#1310). Without the
// landing time a flight reads as an open-ended block and the reader
// cannot tell how much of the day is left for anything else.