From 3bd17ce1f98041c1c4ee6c7ed3c3631421c12d3e Mon Sep 17 00:00:00 2001 From: jubnl Date: Sat, 18 Jul 2026 19:38:35 +0200 Subject: [PATCH] fix(client): convert mixed-currency day/trip cost totals instead of mislabeling raw sums (#1561) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Day headers, the plan sidebar footer and the PDF day/cover totals summed raw place prices across currencies and labeled the result with a single currency — a $2,730.27 hotel on a NOK trip read as "2730 NOK". Totals now convert every amount into a base currency via the existing frankfurter rates (sidebar: the user's display currency, falling back to the trip's; PDF: the trip currency, resolved once before rendering so the document is consistent), marked with "≈" when a conversion happened. When a rate is unavailable (offline, blocked egress, unknown code) they fall back to an honest per-currency breakdown ("2 500 kr + $2,730.27") instead of folding foreign amounts into a mislabeled number. All-same-currency trips make no FX request, so offline PDF export keeps working. Also swaps the place inspector's hardcoded € chip icon for a neutral one and formats the price via formatMoney in the place's own currency. Note: day-header totals move from "50 EUR" to Intl formatting ("50 €"). --- client/src/components/PDF/TripPDF.test.ts | 62 +++++++++++++++ client/src/components/PDF/TripPDF.tsx | 35 +++++--- .../Planner/DayPlanSidebar.test.tsx | 50 ++++++++++++ .../src/components/Planner/DayPlanSidebar.tsx | 32 +++++--- .../Planner/DayPlanSidebarFooter.tsx | 11 +-- .../Planner/PlaceInspector.test.tsx | 19 ++++- .../src/components/Planner/PlaceInspector.tsx | 9 ++- client/src/hooks/useExchangeRates.test.ts | 47 +++++++++++ client/src/hooks/useExchangeRates.ts | 56 +++++++++---- client/src/utils/formatters.test.ts | 79 ++++++++++++++++++- client/src/utils/formatters.ts | 66 +++++++++++++++- client/tests/unit/utils/formatters.test.ts | 41 ++++++++-- 12 files changed, 452 insertions(+), 55 deletions(-) create mode 100644 client/src/hooks/useExchangeRates.test.ts diff --git a/client/src/components/PDF/TripPDF.test.ts b/client/src/components/PDF/TripPDF.test.ts index 25ae66311..3867d9850 100644 --- a/client/src/components/PDF/TripPDF.test.ts +++ b/client/src/components/PDF/TripPDF.test.ts @@ -3,6 +3,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { http, HttpResponse } from 'msw' import { downloadTripPDF } from './TripPDF' import { server } from '../../../tests/helpers/msw/server' +import { clearExchangeRateCache } from '../../hooks/useExchangeRates' // ── Helpers ─────────────────────────────────────────────────────────────────── @@ -50,7 +51,11 @@ beforeEach(() => { http.get('/api/pdf-sections/:tripId', () => HttpResponse.json({ sections: [] }) ), + // Mixed-currency exports fetch FX rates; keep the suite hermetic. + http.get('https://api.frankfurter.dev/v2/rates', () => HttpResponse.json([])), ) + // The FX cache is module-level and would leak rates between tests in this file. + clearExchangeRateCache() }) afterEach(() => { @@ -329,6 +334,63 @@ describe('downloadTripPDF', () => { expect(iframe!.srcdoc).toContain('￥1,500') }) + it('FE-COMP-TRIPPDF-016d: converts foreign-currency prices into the trip currency for day and cover totals (#1561)', async () => { + server.use(http.get('https://api.frankfurter.dev/v2/rates', ({ request }) => { + expect(new URL(request.url).searchParams.get('base')).toBe('NOK') + return HttpResponse.json([{ quote: 'USD', rate: 0.1 }]) // 1 NOK = 0.1 USD + })) + const mixedArgs = { + ...richArgs, + trip: { ...richArgs.trip, currency: 'NOK' }, + assignments: { + '10': [ + { ...assignmentForDay, place: { ...placeWithDetails, currency: 'USD', price: '273' } }, + { ...assignmentForDay, id: 201, place: { ...placeWithDetails, id: 101, name: 'Museum', currency: null, price: '2500' } }, + ], + } as any, + } + await downloadTripPDF(mixedArgs) + const srcdoc = getIframe()!.srcdoc.replace(/[\u00A0\u202F]/g, ' ') + // 2500 NOK + 273 USD / 0.1 = 5230 NOK, marked approximate, in day header AND cover stat. + expect(srcdoc).toContain('≈ 5 230,00 kr') + expect((srcdoc.match(/≈ 5 230,00 kr/g) || []).length).toBeGreaterThanOrEqual(2) + }) + + it('FE-COMP-TRIPPDF-016e: falls back to per-currency breakdowns when the FX fetch fails (#1561)', async () => { + server.use(http.get('https://api.frankfurter.dev/v2/rates', () => HttpResponse.error())) + const mixedArgs = { + ...richArgs, + trip: { ...richArgs.trip, currency: 'NOK' }, + assignments: { + '10': [ + { ...assignmentForDay, place: { ...placeWithDetails, currency: 'USD', price: '2730.27' } }, + { ...assignmentForDay, id: 201, place: { ...placeWithDetails, id: 101, name: 'Museum', currency: null, price: '2500' } }, + ], + } as any, + } + // Export still resolves — a dead FX endpoint must never break the PDF. + await expect(downloadTripPDF(mixedArgs)).resolves.not.toThrow() + const srcdoc = getIframe()!.srcdoc.replace(/[\u00A0\u202F]/g, ' ') + // Honest breakdown, base currency first; the USD amount is never NOK-labeled. + expect(srcdoc).toContain('2 500,00 kr + $2,730.27') + expect(srcdoc).not.toContain('≈') + expect(srcdoc).not.toMatch(/5 ?230/) + }) + + it('FE-COMP-TRIPPDF-016f: an all-same-currency trip makes no FX request', async () => { + let fxCalled = false + server.use(http.get('https://api.frankfurter.dev/v2/rates', () => { + fxCalled = true + return HttpResponse.json([]) + })) + await downloadTripPDF({ ...richArgs, trip: { ...richArgs.trip, currency: 'EUR' } }) + expect(fxCalled).toBe(false) + // Totals render exactly as before for the single-currency case. + const srcdoc = getIframe()!.srcdoc.replace(/[\u00A0\u202F]/g, ' ') + expect(srcdoc).toContain('15,00 €') + expect(srcdoc).not.toContain('≈') + }) + it('FE-COMP-TRIPPDF-017: renders trip description on cover', async () => { await downloadTripPDF(richArgs) const iframe = getIframe() diff --git a/client/src/components/PDF/TripPDF.tsx b/client/src/components/PDF/TripPDF.tsx index 32155aded..98745bd8e 100644 --- a/client/src/components/PDF/TripPDF.tsx +++ b/client/src/components/PDF/TripPDF.tsx @@ -5,7 +5,8 @@ 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 { formatMoney, splitReservationDateTime } from '../../utils/formatters' +import { formatMoney, formatMoneySum, splitReservationDateTime, type MoneyEntry } from '../../utils/formatters' +import { fetchExchangeRates } from '../../hooks/useExchangeRates' import { getFlightLegs, getTrainLegs } from '../../utils/flightLegs' function renderLucideIcon(icon:LucideIcon, props = {}) { @@ -92,9 +93,15 @@ function longDateRange(days, locale) { return `${f.toLocaleDateString(locale, { day: 'numeric', month: 'long', timeZone: 'UTC' })} – ${l.toLocaleDateString(locale, { day: 'numeric', month: 'long', year: 'numeric', timeZone: 'UTC' })}` } -function dayCost(assignments, dayId, locale, currency) { - const total = (assignments[String(dayId)] || []).reduce((s, a) => s + (parseFloat(a.place?.price) || 0), 0) - return total > 0 ? formatMoney(total, currency, locale) : null +// Day totals render in the trip's currency; foreign-currency place prices are +// converted via the pre-fetched rates, or listed per-currency when rates are +// unavailable (#1561). +function dayCost(assignments, dayId, locale, tripCurrency, rates) { + const entries: MoneyEntry[] = (assignments[String(dayId)] || []).map(a => ({ + amount: parseFloat(a.place?.price) || 0, + currency: a.place?.currency || tripCurrency, + })) + return formatMoneySum(entries, tripCurrency, locale || 'en', rates) } // Pre-fetch place photos for all assigned places. @@ -160,8 +167,18 @@ export async function downloadTripPDF({ trip, days, places, assignments, categor const totalAssigned = new Set( Object.values(assignments || {}).flatMap(a => a.map(x => x.place?.id)).filter(Boolean) ).size - const totalCost = Object.values(assignments || {}) - .flatMap(a => a).reduce((s, a) => s + (Number(a.place?.price) || 0), 0) + // The PDF is a trip-scoped, shareable document, so totals stay in the trip's + // own currency. Rates are resolved ONCE before any HTML is built so the cover + // stat and every day header agree; all-same-currency trips skip the FX fetch + // entirely (offline export keeps working), and a failed fetch degrades to + // per-currency breakdowns instead of mislabeled sums (#1561). + const tripCur = (trip?.currency || 'EUR').toUpperCase() + const allCostEntries: MoneyEntry[] = Object.values(assignments || {}) + .flatMap(a => a) + .map(a => ({ amount: Number(a.place?.price) || 0, currency: a.place?.currency || tripCur })) + const needsFx = allCostEntries.some(e => e.amount > 0 && e.currency.toUpperCase() !== tripCur) + 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 @@ -204,7 +221,7 @@ export async function downloadTripPDF({ trip, days, places, assignments, categor const daysHtml = sorted.map((day, di) => { const assigned = assignments[String(day.id)] || [] const notes = (dayNotes || []).filter(n => n.day_id === day.id) - const cost = dayCost(assignments, day.id, loc, trip.currency) + 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) const dayReservations = pdfGetTransportForDay(day.id) @@ -624,8 +641,8 @@ export async function downloadTripPDF({ trip, days, places, assignments, categor
${totalAssigned}
${escHtml(tr('pdf.planned'))}
- ${totalCost > 0 ? `
-
${formatMoney(totalCost, trip.currency, loc)}
+ ${totalCostLabel ? `
+
${totalCostLabel}
${escHtml(tr('pdf.costLabel'))}
` : ''}
diff --git a/client/src/components/Planner/DayPlanSidebar.test.tsx b/client/src/components/Planner/DayPlanSidebar.test.tsx index 42c15af61..971ff1b23 100644 --- a/client/src/components/Planner/DayPlanSidebar.test.tsx +++ b/client/src/components/Planner/DayPlanSidebar.test.tsx @@ -1,6 +1,9 @@ // FE-PLANNER-DAYPLAN-001 to FE-PLANNER-DAYPLAN-042 import { render, screen, waitFor, fireEvent } from '../../../tests/helpers/render' import userEvent from '@testing-library/user-event' +import { http, HttpResponse } from 'msw' +import { server } from '../../../tests/helpers/msw/server' +import { clearExchangeRateCache } from '../../hooks/useExchangeRates' import { useAuthStore } from '../../store/authStore' import { useTripStore } from '../../store/tripStore' import { useSettingsStore } from '../../store/settingsStore' @@ -130,6 +133,9 @@ beforeEach(() => { vi.clearAllMocks() sessionStorage.clear() localStorage.clear() + // Cost totals fetch FX rates for their base currency; keep the suite hermetic. + clearExchangeRateCache() + server.use(http.get('https://api.frankfurter.dev/v2/rates', () => HttpResponse.json([]))) // Reset mutable day-notes state mockDayNotesState.noteUi = {} mockDayNotesState.dayNotes = {} @@ -602,6 +608,50 @@ describe('DayPlanSidebar', () => { expect(screen.getByText('Total Cost')).toBeInTheDocument() }) + it('FE-PLANNER-DAYPLAN-037b: converts foreign-currency prices into the display currency (#1561)', async () => { + server.use(http.get('https://api.frankfurter.dev/v2/rates', ({ request }) => { + expect(new URL(request.url).searchParams.get('base')).toBe('USD') + return HttpResponse.json([{ quote: 'NOK', rate: 10 }]) // 10 NOK per 1 USD + })) + seedStore(useSettingsStore, { settings: { time_format: '24h', default_currency: 'USD' } } as any) + const placeUsd = buildPlace({ id: 1, name: 'Hotel', price: 25, currency: 'USD' } as any) + const placeNok = buildPlace({ id: 2, name: 'Museum', price: 250, currency: null } as any) // implicit trip currency + const day = buildDay({ id: 10, date: '2025-06-01', title: 'Day 1' }) + render() + // $25 + 250 NOK / 10 = $50, marked as approximate; footer and day header agree. + await waitFor(() => expect(screen.getByText('≈ $50.00')).toBeInTheDocument()) + expect(screen.getByText('≈ $50')).toBeInTheDocument() + }) + + it('FE-PLANNER-DAYPLAN-037c: falls back to a per-currency breakdown when rates are unavailable (#1561)', async () => { + server.use(http.get('https://api.frankfurter.dev/v2/rates', () => HttpResponse.error())) + const placeUsd = buildPlace({ id: 1, name: 'Hotel', price: 2730.27, currency: 'USD' } as any) + const placeNok = buildPlace({ id: 2, name: 'Museum', price: 2500, currency: 'NOK' } as any) + const day = buildDay({ id: 10, date: '2025-06-01', title: 'Day 1' }) + render() + // The USD amount must never be folded into a NOK-labeled number (the #1561 bug): + // footer keeps each currency separate, base (NOK) first. + await waitFor(() => expect(screen.getByText(/kr.*\+.*\$2,730\.27|2\s?500,00\s?kr.*\+/)).toBeInTheDocument()) + expect(screen.queryByText(/≈/)).toBeNull() + expect(screen.queryByText(/5\s?230/)).toBeNull() + }) + // ── Route tools (Optimize / Google Maps) ──────────────────────────────── it('FE-PLANNER-DAYPLAN-038: optimize button calls onReorder with 3 geo-places', async () => { diff --git a/client/src/components/Planner/DayPlanSidebar.tsx b/client/src/components/Planner/DayPlanSidebar.tsx index bf0e50818..9e2d45bab 100644 --- a/client/src/components/Planner/DayPlanSidebar.tsx +++ b/client/src/components/Planner/DayPlanSidebar.tsx @@ -28,8 +28,9 @@ import { getTransportForDay as _getTransportForDay, getMergedItems as _getMergedItems, type MergedItem, } from '../../utils/dayMerge' -import { formatDate, formatTime, dayTotalCost, splitReservationDateTime } from '../../utils/formatters' +import { formatDate, formatTime, dayTotalCost, formatMoneySum, splitReservationDateTime } from '../../utils/formatters' import { useDayNotes } from '../../hooks/useDayNotes' +import { useExchangeRates } from '../../hooks/useExchangeRates' import { RES_ICONS, getNoteIcon } from './DayPlanSidebar.constants' import { RouteConnector, HotelRouteConnector } from './DayPlanSidebarRouteConnector' import { MobileAddPlaceButton } from './DayPlanSidebarMobileAddPlaceButton' @@ -240,6 +241,12 @@ function useDayPlanSidebar(props: DayPlanSidebarProps) { }, [selectedAssignmentId, selectedPlaceId]) const currency = trip?.currency || 'EUR' + // Cost totals render in the user's display currency (falling back to the + // trip's), converting foreign-currency place prices via live FX rates — + // same base resolution as BookingCostsSection / the Costs panel (#1561). + const displayCurrency = useSettingsStore(s => s.settings.default_currency) + const costBase = (displayCurrency || currency).toUpperCase() + const { rates: fxRates } = useExchangeRates(costBase) // Drag-Daten aus dataTransfer, Ref oder window lesen (dataTransfer geht bei Re-Render verloren) const getDragData = (e) => { @@ -969,10 +976,13 @@ function useDayPlanSidebar(props: DayPlanSidebarProps) { setDraggingId(null) } - const totalCost = useMemo(() => days.reduce((s, d) => { - const da = assignments[String(d.id)] || [] - return s + da.reduce((s2, a) => s2 + (Number(a.place?.price) || 0), 0) - }, 0), [days, assignments]) + const totalCostLabel = useMemo(() => { + const entries = days.flatMap(d => (assignments[String(d.id)] || []).map(a => ({ + amount: Number(a.place?.price) || 0, + currency: a.place?.currency || currency, + }))) + return formatMoneySum(entries, costBase, locale, fxRates) + }, [days, assignments, currency, costBase, locale, fxRates]) // Bester verfügbarer Standort für Wetter: zugewiesene Orte zuerst, dann beliebiger Reiseort const anyGeoAssignment = Object.values(assignments).flatMap(da => da).find(a => a.place?.lat && a.place?.lng) @@ -1101,6 +1111,8 @@ function useDayPlanSidebar(props: DayPlanSidebarProps) { initedTransportIds, lastAutoScrolledIdRef, currency, + costBase, + fxRates, getDragData, prevDayCount, toggleDay, @@ -1125,7 +1137,7 @@ function useDayPlanSidebar(props: DayPlanSidebarProps) { handleOptimize, handleDropOnDay, handleDropOnRow, - totalCost, + totalCostLabel, anyGeoAssignment, anyGeoPlace, expandedRouteDayIds, @@ -1268,6 +1280,8 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar(props: DayPlanSidebarP initedTransportIds, lastAutoScrolledIdRef, currency, + costBase, + fxRates, getDragData, prevDayCount, toggleDay, @@ -1292,7 +1306,7 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar(props: DayPlanSidebarP handleOptimize, handleDropOnDay, handleDropOnRow, - totalCost, + totalCostLabel, anyGeoAssignment, anyGeoPlace, expandedRouteDayIds, @@ -1335,7 +1349,7 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar(props: DayPlanSidebarP const isSelected = selectedDayId === day.id const isExpanded = expandedDays.has(day.id) const da = getDayAssignments(day.id) - const cost = dayTotalCost(day.id, assignments, currency) + const cost = dayTotalCost(day.id, assignments, costBase, currency, locale, fxRates) const formattedDate = formatDate(day.date, locale) const loc = da.find(a => a.place?.lat && a.place?.lng) // Route tools normally need 2+ stops, but a single located place is still @@ -2496,7 +2510,7 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar(props: DayPlanSidebarP /> {/* Budget-Fußzeile */} - + ) diff --git a/client/src/components/Planner/DayPlanSidebarFooter.tsx b/client/src/components/Planner/DayPlanSidebarFooter.tsx index 6c965ccf4..789ebfbf0 100644 --- a/client/src/components/Planner/DayPlanSidebarFooter.tsx +++ b/client/src/components/Planner/DayPlanSidebarFooter.tsx @@ -1,17 +1,14 @@ -import { currencyDecimals } from '../../utils/formatters' - interface DayPlanSidebarFooterProps { - totalCost: number - currency: string + totalCostLabel: string | null t: (key: string, params?: Record) => string } -export function DayPlanSidebarFooter({ totalCost, currency, t }: DayPlanSidebarFooterProps) { - if (!(totalCost > 0)) return null +export function DayPlanSidebarFooter({ totalCostLabel, t }: DayPlanSidebarFooterProps) { + if (!totalCostLabel) return null return (
{t('dayplan.totalCost')} - {totalCost.toFixed(currencyDecimals(currency))} {currency} + {totalCostLabel}
) } diff --git a/client/src/components/Planner/PlaceInspector.test.tsx b/client/src/components/Planner/PlaceInspector.test.tsx index 38c0ae340..8cf77b48a 100644 --- a/client/src/components/Planner/PlaceInspector.test.tsx +++ b/client/src/components/Planner/PlaceInspector.test.tsx @@ -443,7 +443,24 @@ describe('PlaceInspector', () => { it('FE-PLANNER-INSPECTOR-032: price chip shown when place.price > 0', () => { const p = buildPlace({ id: 300, price: 15, currency: 'EUR' } as any); render(); - expect(screen.getByText(/15 EUR/)).toBeTruthy(); + // formatMoney renders in the currency's home convention (de-DE for EUR). + expect(screen.getByText(/15,00/)).toBeTruthy(); + }); + + it('FE-PLANNER-INSPECTOR-032b: price chip formats in the place currency with a neutral icon (#1561)', () => { + const p = buildPlace({ id: 300, price: 15, currency: 'USD' } as any); + render(); + expect(screen.getByText('$15.00')).toBeTruthy(); + // The chip icon must be currency-neutral, not the euro glyph. + expect(document.querySelector('.lucide-euro')).toBeNull(); + expect(document.querySelector('.lucide-banknote')).not.toBeNull(); + }); + + it('FE-PLANNER-INSPECTOR-032c: a currency-less price falls back to the trip currency (#1561)', () => { + seedStore(useTripStore, { trip: buildTrip({ id: 1, currency: 'NOK' }) } as any); + const p = buildPlace({ id: 300, price: 250, currency: null } as any); + render(); + expect(screen.getByText(/250,00\s?kr|kr\s?250,00/)).toBeTruthy(); }); // ── Phone number ─────────────────────────────────────────────────────────── diff --git a/client/src/components/Planner/PlaceInspector.tsx b/client/src/components/Planner/PlaceInspector.tsx index 35f6eccbf..b503688c6 100644 --- a/client/src/components/Planner/PlaceInspector.tsx +++ b/client/src/components/Planner/PlaceInspector.tsx @@ -4,7 +4,7 @@ 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, Map as MapIcon, Users, Mountain, TrendingUp, Bookmark, BookmarkCheck, Copy } from 'lucide-react' +import { X, Clock, MapPin, ExternalLink, Phone, Banknote, Edit2, Trash2, Plus, Minus, ChevronDown, ChevronUp, FileText, Upload, File, FileImage, Star, Navigation, Map as MapIcon, Users, Mountain, TrendingUp, Bookmark, BookmarkCheck, Copy } from 'lucide-react' import PlaceAvatar from '../shared/PlaceAvatar' import GuestBadge from '../shared/GuestBadge' import StatusBadge from '../Collections/StatusBadge' @@ -20,7 +20,8 @@ import { usePluginStore } from '../../store/pluginStore' import PluginFrame from '../Plugins/PluginFrame' import type { Place, Category, Day, Assignment, Reservation, TripFile, AssignmentsMap } from '../../types' import type { CollectionStatus } from '@trek/shared' -import { splitReservationDateTime, formatTime } from '../../utils/formatters' +import { splitReservationDateTime, formatTime, formatMoney } from '../../utils/formatters' +import { useTripStore } from '../../store/tripStore' import { formatDistance, formatElevation } from '../../utils/units' import { getGoogleMapsUrlForPlace } from './placeGoogleMaps' import { getOpenStreetMapUrlForPlace } from './placeOpenStreetMap' @@ -160,6 +161,8 @@ export default function PlaceInspector({ return () => { cancelled = true } }, [placeIdForDetails]) const { t, locale, language } = useTranslation() + // Currency-less prices mean "the trip's currency"; null in collection mode (EUR fallback below). + const tripCurrency = useTripStore(s => s.trip?.currency) const toast = useToast() const timeFormat = useSettingsStore(s => s.settings.time_format) || '24h' const distanceUnit = useSettingsStore(s => s.settings.distance_unit) || 'metric' @@ -330,7 +333,7 @@ export default function PlaceInspector({ ) })()} {place.price > 0 && ( - } text={`${place.price} ${place.currency || '€'}`} color="#059669" bg="#ecfdf5" /> + } text={formatMoney(Number(place.price) || 0, place.currency || tripCurrency || 'EUR', locale)} color="#059669" bg="#ecfdf5" /> )} diff --git a/client/src/hooks/useExchangeRates.test.ts b/client/src/hooks/useExchangeRates.test.ts new file mode 100644 index 000000000..de6d3904c --- /dev/null +++ b/client/src/hooks/useExchangeRates.test.ts @@ -0,0 +1,47 @@ +import { describe, it, expect, beforeEach } from 'vitest' +import { http, HttpResponse } from 'msw' +import { server } from '../../tests/helpers/msw/server' +import { fetchExchangeRates, clearExchangeRateCache } from './useExchangeRates' + +const FX_URL = 'https://api.frankfurter.dev/v2/rates' + +// Contract tests for the plain fetcher the PDF export relies on: it must never +// reject, and "no usable rates" must come back as null (→ breakdown fallback), +// never as a half-filled object. +describe('fetchExchangeRates (#1561)', () => { + beforeEach(() => { + clearExchangeRateCache() + }) + + it('fetches, seeds the base self-rate, and caches', async () => { + let calls = 0 + server.use(http.get(FX_URL, () => { + calls++ + return HttpResponse.json([{ quote: 'USD', rate: 0.095 }, { quote: 'bogus' }]) + })) + const rates = await fetchExchangeRates('nok') + expect(rates).toEqual({ NOK: 1, USD: 0.095 }) + // fresh cache short-circuits the second call + expect(await fetchExchangeRates('NOK')).toEqual(rates) + expect(calls).toBe(1) + }) + + it('returns null on failure with no cache', async () => { + server.use(http.get(FX_URL, () => HttpResponse.error())) + expect(await fetchExchangeRates('NOK')).toBeNull() + }) + + it('returns null for a non-array body', async () => { + server.use(http.get(FX_URL, () => HttpResponse.json({ message: 'not found' }))) + expect(await fetchExchangeRates('NOK')).toBeNull() + }) + + it('falls back to a stale localStorage cache when the fetch fails', async () => { + localStorage.setItem('trek_fx_NOK', JSON.stringify({ + rates: { NOK: 1, USD: 0.1 }, + ts: Date.now() - 24 * 60 * 60 * 1000, // expired + })) + server.use(http.get(FX_URL, () => HttpResponse.error())) + expect(await fetchExchangeRates('NOK')).toEqual({ NOK: 1, USD: 0.1 }) + }) +}) diff --git a/client/src/hooks/useExchangeRates.ts b/client/src/hooks/useExchangeRates.ts index f1f5c3fd0..46a40ed93 100644 --- a/client/src/hooks/useExchangeRates.ts +++ b/client/src/hooks/useExchangeRates.ts @@ -24,6 +24,44 @@ function readCache(base: string): { rates: Record; ts: number } return null } +/** + * Plain-function twin of the hook, for non-React callers (PDF export). Never + * rejects: a fresh cache short-circuits, otherwise it fetches and caches; on + * any failure it returns the stale cache if one exists, else null ("no rates" — + * callers fall back to per-currency breakdowns rather than converting). + */ +export async function fetchExchangeRates(base: string): Promise | null> { + const upper = (base || 'EUR').toUpperCase() + const cached = readCache(upper) + if (cached && Date.now() - cached.ts < TTL_MS) return cached.rates + try { + const d = await fetch(`https://api.frankfurter.dev/v2/rates?base=${encodeURIComponent(upper)}`) + .then(r => r.json()) as Array<{ quote?: string; rate?: number }> + if (!Array.isArray(d)) return cached?.rates ?? null + // Frankfurter omits the base's own self-rate, so seed it with `base = 1`. + const rates: Record = { [upper]: 1 } + for (const r of d) { + if (r && typeof r.quote === 'string' && typeof r.rate === 'number') rates[r.quote] = r.rate + } + const entry = { rates, ts: Date.now() } + mem.set(upper, entry) + try { localStorage.setItem('trek_fx_' + upper, JSON.stringify(entry)) } catch { /* ignore */ } + return rates + } catch { + return cached?.rates ?? null // offline → stale beats nothing + } +} + +/** Test-only: the module-level cache outlives a vitest file's individual tests. */ +export function clearExchangeRateCache(): void { + mem.clear() + try { + for (const k of Object.keys(localStorage)) { + if (k.startsWith('trek_fx_')) localStorage.removeItem(k) + } + } catch { /* ignore */ } +} + export function useExchangeRates(base: string) { const upper = (base || 'EUR').toUpperCase() const [rates, setRates] = useState | null>(() => readCache(upper)?.rates ?? null) @@ -33,21 +71,9 @@ export function useExchangeRates(base: string) { if (cached) setRates(cached.rates) if (cached && Date.now() - cached.ts < TTL_MS) return let cancelled = false - fetch(`https://api.frankfurter.dev/v2/rates?base=${encodeURIComponent(upper)}`) - .then(r => r.json()) - .then((d: Array<{ quote?: string; rate?: number }>) => { - if (cancelled || !Array.isArray(d)) return - // Frankfurter omits the base's own self-rate, so seed it with `base = 1`. - const rates: Record = { [upper]: 1 } - for (const r of d) { - if (r && typeof r.quote === 'string' && typeof r.rate === 'number') rates[r.quote] = r.rate - } - const entry = { rates, ts: Date.now() } - mem.set(upper, entry) - try { localStorage.setItem('trek_fx_' + upper, JSON.stringify(entry)) } catch { /* ignore */ } - setRates(rates) - }) - .catch(() => { /* offline → keep cached/identity */ }) + fetchExchangeRates(upper).then(r => { + if (!cancelled && r) setRates(r) + }) return () => { cancelled = true } }, [upper]) diff --git a/client/src/utils/formatters.test.ts b/client/src/utils/formatters.test.ts index 81b0a4677..04a8a6eb9 100644 --- a/client/src/utils/formatters.test.ts +++ b/client/src/utils/formatters.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from 'vitest' -import { splitReservationDateTime, resolveDayId, formatMoney, currencyDecimals } from './formatters' +import { splitReservationDateTime, resolveDayId, formatMoney, formatMoneySum, currencyDecimals } from './formatters' import { CURRENCIES, SYMBOLS, currenciesWith } from '../components/Budget/BudgetPanel.constants' import type { Day } from '../types' @@ -83,6 +83,83 @@ describe('currenciesWith (legacy safeguard)', () => { }) }) +describe('formatMoneySum (#1561)', () => { + // Intl money strings use non-breaking / narrow no-break spaces; normalize for assertions. + const norm = (s: string | null) => s?.replace(/[\u00A0\u202F]/g, ' ') ?? null + + it('returns null for empty input and for zero/negative/non-finite amounts', () => { + expect(formatMoneySum([], 'EUR', 'en')).toBeNull() + expect(formatMoneySum([{ amount: 0, currency: 'EUR' }], 'EUR', 'en')).toBeNull() + expect(formatMoneySum([{ amount: -5, currency: 'EUR' }], 'EUR', 'en')).toBeNull() + expect(formatMoneySum([{ amount: NaN, currency: 'EUR' }], 'EUR', 'en')).toBeNull() + }) + + it('sums a single-currency set without rates and without an ≈ marker', () => { + const out = formatMoneySum( + [{ amount: 20, currency: 'USD' }, { amount: 30, currency: 'usd' }], + 'USD', 'en', + ) + expect(out).toBe('$50.00') + }) + + it('converts foreign amounts into the base when rates cover them', () => { + // rates[X] = units of X per 1 base (frankfurter): 10 NOK = 1 USD here. + const out = formatMoneySum( + [{ amount: 25, currency: 'USD' }, { amount: 250, currency: 'NOK' }], + 'USD', 'en', { NOK: 10 }, + ) + expect(out).toBe('≈ $50.00') + }) + + it('falls back to a per-currency breakdown when rates are missing', () => { + const entries = [ + { amount: 2500, currency: 'NOK' }, + { amount: 2730.27, currency: 'USD' }, + ] + for (const rates of [undefined, null, {}, { USD: 0 }]) { + const out = norm(formatMoneySum(entries, 'NOK', 'en', rates)) + expect(out).toContain(' + ') + expect(out).toContain('$2,730.27') + expect(out).not.toContain('≈') + // the foreign amount must never be folded into a base-labeled number + expect(out).not.toMatch(/5 ?230/) + } + }) + + it('orders the breakdown base-first, then foreign currencies by code', () => { + const out = norm(formatMoneySum( + [ + { amount: 1, currency: 'USD' }, + { amount: 2, currency: 'EUR' }, + { amount: 3, currency: 'NOK' }, + ], + 'NOK', 'en', + ))! + expect(out.indexOf('kr')).toBeLessThan(out.indexOf('€')) + expect(out.indexOf('€')).toBeLessThan(out.indexOf('$')) + }) + + it('renders a lone foreign amount honestly in its own currency', () => { + expect(formatMoneySum([{ amount: 2730.27, currency: 'USD' }], 'NOK', 'en')).toBe('$2,730.27') + }) + + it('applies opts.decimals to every part', () => { + expect(formatMoneySum([{ amount: 50.4, currency: 'USD' }], 'USD', 'en', null, { decimals: 0 })).toBe('$50') + const breakdown = norm(formatMoneySum( + [{ amount: 50.4, currency: 'USD' }, { amount: 10.2, currency: 'NOK' }], + 'USD', 'en', null, { decimals: 0 }, + )) + expect(breakdown).toContain('$50') + expect(breakdown).toMatch(/10\s?kr|kr\s?10/) + }) + + it('tolerates currency codes unknown to Intl', () => { + const out = formatMoneySum([{ amount: 12, currency: 'XYZ' }], 'XYZ', 'en') + expect(out).toContain('XYZ') + expect(out).toContain('12') + }) +}) + describe('splitReservationDateTime', () => { it('parses full ISO datetime', () => { expect(splitReservationDateTime('2026-06-25T10:00')).toEqual({ date: '2026-06-25', time: '10:00' }) diff --git a/client/src/utils/formatters.ts b/client/src/utils/formatters.ts index cc6abd3dd..a4b5b0237 100644 --- a/client/src/utils/formatters.ts +++ b/client/src/utils/formatters.ts @@ -164,8 +164,66 @@ export function resolveDayId(days: Day[], value: string | null | undefined): Day return best } -export function dayTotalCost(dayId: number, assignments: AssignmentsMap, currency: string): string | null { - const da = assignments[String(dayId)] || [] - const total = da.reduce((s, a) => s + (parseFloat(String(a.place?.price ?? '')) || 0), 0) - return total > 0 ? `${total.toFixed(0)} ${currency}` : null +export type MoneyEntry = { amount: number; currency: string } + +/** + * Formats a sum of amounts that may span several currencies. Callers must resolve + * each entry's currency beforehand (`place.currency || trip.currency`) — a null + * place currency means the TRIP currency, which is not necessarily `base`. + * + * With usable rates (frankfurter convention: rates[X] = units of X per 1 base), + * everything is converted into `base` and prefixed with "≈" when a conversion + * actually happened. When any needed rate is missing, falls back to an honest + * per-currency breakdown ("2 500 kr + $2,730.27") — never a raw foreign amount + * labeled with the base currency. + */ +export function formatMoneySum( + entries: MoneyEntry[], + base: string, + locale: string, + rates?: Record | null, + opts?: { decimals?: number }, +): string | null { + const baseCur = (base || 'EUR').toUpperCase() + const groups = new Map() + for (const e of entries) { + if (!Number.isFinite(e.amount) || e.amount <= 0) continue + const cur = (e.currency || baseCur).toUpperCase() + groups.set(cur, (groups.get(cur) || 0) + e.amount) + } + if (groups.size === 0) return null + + const foreign = [...groups.keys()].filter(c => c !== baseCur) + if (foreign.length === 0) return formatMoney(groups.get(baseCur)!, baseCur, locale, opts) + + if (foreign.every(c => (rates?.[c] ?? 0) > 0)) { + const total = [...groups.entries()].reduce( + (s, [cur, amount]) => s + (cur === baseCur ? amount : amount / rates![cur]), + 0, + ) + return `≈ ${formatMoney(total, baseCur, locale, opts)}` + } + + // Breakdown: base first, then the rest in stable code order. + const parts = [ + ...(groups.has(baseCur) ? [baseCur] : []), + ...foreign.sort(), + ] + return parts.map(cur => formatMoney(groups.get(cur)!, cur, locale, opts)).join(' + ') +} + +export function dayTotalCost( + dayId: number, + assignments: AssignmentsMap, + base: string, + tripCurrency: string, + locale: string, + rates?: Record | null, +): string | null { + const da = assignments[String(dayId)] || [] + const entries = da.map(a => ({ + amount: parseFloat(String(a.place?.price ?? '')) || 0, + currency: a.place?.currency || tripCurrency, + })) + return formatMoneySum(entries, base, locale, rates, { decimals: 0 }) } diff --git a/client/tests/unit/utils/formatters.test.ts b/client/tests/unit/utils/formatters.test.ts index 961070028..4f6cee194 100644 --- a/client/tests/unit/utils/formatters.test.ts +++ b/client/tests/unit/utils/formatters.test.ts @@ -63,8 +63,11 @@ describe('formatTime', () => { }); describe('dayTotalCost', () => { + // Intl money strings use non-breaking / narrow no-break spaces; normalize for assertions. + const norm = (s: string | null) => s?.replace(/[\u00A0\u202F]/g, ' ') ?? null; + it('returns null when there are no assignments', () => { - expect(dayTotalCost(1, {}, 'EUR')).toBeNull(); + expect(dayTotalCost(1, {}, 'EUR', 'EUR', 'en')).toBeNull(); }); it('returns null when no places have prices', () => { @@ -73,7 +76,7 @@ describe('dayTotalCost', () => { { id: 1, day_id: 1, order_index: 0, notes: null, place: { id: 1, trip_id: 1, name: 'P', lat: null, lng: null, description: null, address: null, category_id: null, icon: null, price: null, image_url: null, google_place_id: null, osm_id: null, route_geometry: null, place_time: null, end_time: null, created_at: '' } }, ], }; - expect(dayTotalCost(1, asMap(assignments), 'EUR')).toBeNull(); + expect(dayTotalCost(1, asMap(assignments), 'EUR', 'EUR', 'en')).toBeNull(); }); it('sums prices across assignments', () => { @@ -83,7 +86,7 @@ describe('dayTotalCost', () => { { id: 2, day_id: 1, order_index: 1, notes: null, place: { id: 2, trip_id: 1, name: 'B', lat: null, lng: null, description: null, address: null, category_id: null, icon: null, price: '30', image_url: null, google_place_id: null, osm_id: null, route_geometry: null, place_time: null, end_time: null, created_at: '' } }, ], }; - expect(dayTotalCost(1, asMap(assignments), 'EUR')).toBe('50 EUR'); + expect(norm(dayTotalCost(1, asMap(assignments), 'EUR', 'EUR', 'en'))).toBe('50 €'); }); it('ignores non-numeric price strings', () => { @@ -92,7 +95,7 @@ describe('dayTotalCost', () => { { id: 1, day_id: 1, order_index: 0, notes: null, place: { id: 1, trip_id: 1, name: 'A', lat: null, lng: null, description: null, address: null, category_id: null, icon: null, price: 'free', image_url: null, google_place_id: null, osm_id: null, route_geometry: null, place_time: null, end_time: null, created_at: '' } }, ], }; - expect(dayTotalCost(1, asMap(assignments), 'EUR')).toBeNull(); + expect(dayTotalCost(1, asMap(assignments), 'EUR', 'EUR', 'en')).toBeNull(); }); it('uses the dayId key to look up assignments', () => { @@ -101,7 +104,33 @@ describe('dayTotalCost', () => { { id: 3, day_id: 2, order_index: 0, notes: null, place: { id: 3, trip_id: 1, name: 'C', lat: null, lng: null, description: null, address: null, category_id: null, icon: null, price: '10', image_url: null, google_place_id: null, osm_id: null, route_geometry: null, place_time: null, end_time: null, created_at: '' } }, ], }; - expect(dayTotalCost(1, asMap(assignments), 'USD')).toBeNull(); - expect(dayTotalCost(2, asMap(assignments), 'USD')).toBe('10 USD'); + expect(dayTotalCost(1, asMap(assignments), 'USD', 'USD', 'en')).toBeNull(); + expect(dayTotalCost(2, asMap(assignments), 'USD', 'USD', 'en')).toBe('$10'); + }); + + it('resolves a currency-less place to the trip currency, converting into the base (#1561)', () => { + const assignments = { + '1': [ + { id: 1, day_id: 1, order_index: 0, notes: null, place: { id: 1, trip_id: 1, name: 'A', lat: null, lng: null, description: null, address: null, category_id: null, icon: null, price: '100', currency: null, image_url: null, google_place_id: null, osm_id: null, route_geometry: null, place_time: null, end_time: null, created_at: '' } }, + ], + }; + // base USD, trip NOK: the implicit-NOK price must convert (rates[NOK]=10 per USD)… + expect(dayTotalCost(1, asMap(assignments), 'USD', 'NOK', 'en', { NOK: 10 })).toBe('≈ $10'); + // …and without rates it must show as NOK, never as a raw number labeled USD. + expect(norm(dayTotalCost(1, asMap(assignments), 'USD', 'NOK', 'en'))).toMatch(/100\s?kr|kr\s?100/); + }); + + it('mislabels nothing when a foreign-currency price has no rate (#1561)', () => { + const assignments = { + '1': [ + { id: 1, day_id: 1, order_index: 0, notes: null, place: { id: 1, trip_id: 1, name: 'Hotel', lat: null, lng: null, description: null, address: null, category_id: null, icon: null, price: 2730.27, currency: 'USD', image_url: null, google_place_id: null, osm_id: null, route_geometry: null, place_time: null, end_time: null, created_at: '' } }, + { id: 2, day_id: 1, order_index: 1, notes: null, place: { id: 2, trip_id: 1, name: 'Museum', lat: null, lng: null, description: null, address: null, category_id: null, icon: null, price: 2500, currency: 'NOK', image_url: null, google_place_id: null, osm_id: null, route_geometry: null, place_time: null, end_time: null, created_at: '' } }, + ], + }; + const out = norm(dayTotalCost(1, asMap(assignments), 'NOK', 'NOK', 'en')); + expect(out).toContain(' + '); + expect(out).toMatch(/2 500\s?kr|kr\s?2 500/); + expect(out).toContain('$2,730'); + expect(out).not.toContain('≈'); }); });