From 2601dec2e1133b22b9caa695cea601b8a8c48e7f Mon Sep 17 00:00:00 2001 From: Charles Pizzato <311327716+modernitconsultants@users.noreply.github.com> Date: Sat, 15 Aug 2026 19:21:51 +1000 Subject: [PATCH 1/2] fix(dashboard): there is no default currency MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit lib/money.ts explained, correctly and at length, why a hardcoded currency symbol is wrong for a property trading in anything but dollars — and then exported DEFAULT_CURRENCY = 'USD' two lines later and fell back to it whenever a record carried no code. The comment describing the bug sat directly above it. The consequence is not cosmetic. On a JPY property a missing code renders a real ¥151,110 balance as a dollar figure with two minor units: authoritative-looking and materially wrong, on a live ledger. There is now no fallback. An absent code renders the number plainly — grouped, unsymbolled — which is honest about what is known and odd enough that someone asks. A symbol we invented is worse than no symbol precisely because it looks decided. PropertyContext returns null rather than substituting, including in portfolio mode, where properties may genuinely differ and null IS the answer. That surfaced the more serious half: six pages POST the context currency into request bodies, so the old default could create a deposit, an AR ledger or a rate plan denominated in USD against a yen property — a wrong record, with no error anywhere. requireCurrency() guards those writes, following the existing requirePropertyId idiom. Applied only to mutations that actually send a currency: guarding every mutation beside a propertyId assert would block unrelated actions like a room move, and the bulk-import parser has its own local currencyCode from the pasted rows. Specs assert behaviour, not glyphs — ICU renders JPY as ¥ in some versions and JP¥ in others, and pinning the symbol would fail on a runner upgrade while saying nothing about the defect. Co-Authored-By: Claude Fable 5 --- .../front-desk/GuestDetailsModal.tsx | 12 +++-- .../dashboard/src/context/PropertyContext.tsx | 21 ++++---- apps/dashboard/src/lib/api-helpers.ts | 16 ++++++ apps/dashboard/src/lib/money.spec.ts | 54 +++++++++++++++++++ apps/dashboard/src/lib/money.ts | 39 +++++++------- apps/dashboard/src/pages/Accounting.tsx | 18 ++++--- apps/dashboard/src/pages/Cashier.tsx | 22 ++++---- apps/dashboard/src/pages/Channels.test.tsx | 7 ++- apps/dashboard/src/pages/Commercial.tsx | 7 +-- apps/dashboard/src/pages/Dashboard.tsx | 8 +-- apps/dashboard/src/pages/Folios.tsx | 26 +++++---- apps/dashboard/src/pages/FrontDesk.tsx | 9 ++-- apps/dashboard/src/pages/Groups.tsx | 10 ++-- apps/dashboard/src/pages/HouseAccounts.tsx | 15 +++--- apps/dashboard/src/pages/NightAudit.tsx | 6 ++- apps/dashboard/src/pages/RatePlans.tsx | 7 +-- apps/dashboard/src/pages/Reports.tsx | 22 ++++---- 17 files changed, 196 insertions(+), 103 deletions(-) create mode 100644 apps/dashboard/src/lib/money.spec.ts diff --git a/apps/dashboard/src/components/front-desk/GuestDetailsModal.tsx b/apps/dashboard/src/components/front-desk/GuestDetailsModal.tsx index 0076110f..cee3433d 100644 --- a/apps/dashboard/src/components/front-desk/GuestDetailsModal.tsx +++ b/apps/dashboard/src/components/front-desk/GuestDetailsModal.tsx @@ -1,5 +1,4 @@ import { Link } from 'react-router-dom'; -import { formatMoney } from '../../lib/money'; import { useQuery } from '@tanstack/react-query'; import { useTranslation } from 'react-i18next'; import { ArrowRightLeft, StickyNote, LogIn, LogOut } from 'lucide-react'; @@ -7,6 +6,7 @@ import { api } from '../../lib/api'; import Modal from '../ui/Modal'; import StatusBadge from '../ui/StatusBadge'; import ReservationPartyPanel from '../reservations/ReservationPartyPanel'; +import { formatMoney } from '../../lib/money'; export interface GuestDetailsReservation { id: string; @@ -72,6 +72,7 @@ export default function GuestDetailsModal({ open, reservation, propertyId, + currencyCode, doorPin, onClose, onNotes, @@ -82,6 +83,9 @@ export default function GuestDetailsModal({ }: { open: boolean; reservation: GuestDetailsReservation | null; + /** The property's currency. Null renders unsymbolled rather than guessing — + * this modal shows a real folio balance and must not invent a currency. */ + currencyCode: string | null; propertyId: string; doorPin?: string | null; onClose: () => void; @@ -230,7 +234,7 @@ export default function GuestDetailsModal({ {t('frontDesk.accountSummary')}

- {t('frontDesk.balance')}: {formatMoney(balance)} + {t('frontDesk.balance')}: {formatMoney(balance, currencyCode)}

@@ -248,7 +252,7 @@ export default function GuestDetailsModal({
  • {c.description || '—'} - {formatMoney(c.amount)} + {formatMoney(c.amount, currencyCode)}
    • ))} @@ -269,7 +273,7 @@ export default function GuestDetailsModal({
  • {p.method || '—'} - {formatMoney(p.amount)} + {formatMoney(p.amount, currencyCode)}
    • ))} diff --git a/apps/dashboard/src/context/PropertyContext.tsx b/apps/dashboard/src/context/PropertyContext.tsx index 78fda6f3..fb6ff354 100644 --- a/apps/dashboard/src/context/PropertyContext.tsx +++ b/apps/dashboard/src/context/PropertyContext.tsx @@ -2,7 +2,6 @@ import { createContext, useContext, useState, useEffect, type ReactNode } from ' import { useSearchParams } from 'react-router-dom'; import { api, setPropertyId as setApiPropertyId } from '../lib/api'; import { joinPropertyRoom, leavePropertyRoom } from '../lib/socket'; -import { DEFAULT_CURRENCY, setActiveCurrency } from '../lib/money'; import { PORTFOLIO_MODE_ID, type PropertySummary, @@ -11,8 +10,9 @@ import { interface PropertyContextValue { propertyId: string | null; - /** Active property's ISO 4217 code; falls back to USD before properties load. */ - currencyCode: string; + /** null when unknown — portfolio mode, or a property with no code. Never a + * substituted default: see lib/money.ts. */ + currencyCode: string | null; setPropertyId: (id: string) => void; isPortfolioMode: boolean; properties: PropertySummary[]; @@ -23,7 +23,7 @@ interface PropertyContextValue { const PropertyContext = createContext({ propertyId: null, - currencyCode: DEFAULT_CURRENCY, + currencyCode: null, setPropertyId: () => {}, isPortfolioMode: false, properties: [], @@ -44,11 +44,14 @@ export function PropertyProvider({ children }: { children: ReactNode }) { const isPortfolioMode = propertyId === PORTFOLIO_MODE_ID; // Portfolio mode spans properties that may not share a currency, so it has no - // single answer — fall back rather than assert one property's code over others. + // single answer — and NULL is that answer. Substituting one property's code, + // or a house default, prints a currency nobody chose next to real money. + // formatMoney renders an unsymbolled number when the code is null, which is + // the honest rendering of "we do not know". const currencyCode = (!isPortfolioMode && properties.find((p) => p.id === propertyId)?.currencyCode) || - DEFAULT_CURRENCY; + null; function setPropertyId(id: string) { setPropertyIdState(id); @@ -85,12 +88,6 @@ export function PropertyProvider({ children }: { children: ReactNode }) { // Bootstrap once; propertyId auto-select handled inside the effect. }, []); - // Keep the money formatter's default in step with the active property, the - // same way setApiPropertyId keeps the API client in step above. - useEffect(() => { - setActiveCurrency(currencyCode); - }, [currencyCode]); - useEffect(() => { if (isPortfolioMode) { setApiPropertyId(null); diff --git a/apps/dashboard/src/lib/api-helpers.ts b/apps/dashboard/src/lib/api-helpers.ts index 780b6b56..7c8e3f01 100644 --- a/apps/dashboard/src/lib/api-helpers.ts +++ b/apps/dashboard/src/lib/api-helpers.ts @@ -15,6 +15,22 @@ export function requirePropertyId(propertyId: string | null): asserts propertyId } } +/** + * Assert a currency before WRITING money. + * + * Same idiom as requirePropertyId, and for the same reason. The dashboard used + * to substitute a house default when the active property's code was unknown, + * which meant a deposit or an AR ledger could be created in USD against a + * property trading in yen — a wrong record, silently, with no error anywhere. + * Refusing is the correct outcome: an unknown currency is a reason not to + * write, never a reason to pick one. + */ +export function requireCurrency(currencyCode: string | null): asserts currencyCode is string { + if (!currencyCode) { + throw new Error('No currency for this property — select a single property first'); + } +} + /** Format occupancy rate (0–1 decimal from API) as a percentage string. */ export function formatOccupancyPercent(rate: number | null | undefined): string { if (rate == null) return '—'; diff --git a/apps/dashboard/src/lib/money.spec.ts b/apps/dashboard/src/lib/money.spec.ts new file mode 100644 index 00000000..b552d177 --- /dev/null +++ b/apps/dashboard/src/lib/money.spec.ts @@ -0,0 +1,54 @@ +/** + * Money formatting, tested on the case that put a wrong number on a live folio. + * + * The formatter used to fall back to a hardcoded USD when a record carried no + * currency code, so a real ¥151,110 balance on a JPY property rendered as a + * dollar figure with two decimal places — authoritative-looking and materially + * wrong. The fix is that there is no fallback at all. + * + * These assert BEHAVIOUR rather than exact glyphs: ICU renders JPY as "¥" in + * some versions and "JP¥" in others, and a test that pins the symbol would fail + * on a runner upgrade while telling us nothing about the defect. + */ +import { describe, expect, it } from 'vitest'; + +import { formatMoney } from './money'; + +describe('formatMoney', () => { + it('renders JPY with no minor units', () => { + const out = formatMoney('151110', 'JPY', 'en-US'); + expect(out).toContain('151,110'); + expect(out).not.toContain('.'); // zero-decimal currency + }); + + it('never invents a currency when the code is missing', () => { + // The whole defect: these used to come back as dollar amounts. + for (const missing of [undefined, null, '', ' ']) { + const out = formatMoney('151110', missing, 'en-US'); + expect(out).toBe('151,110'); // grouped, unsymbolled, honest + expect(out).not.toContain('$'); + expect(out).not.toContain('USD'); + } + }); + + it('still renders minor units where the currency has them', () => { + expect(formatMoney('1234.5', 'USD', 'en-US')).toContain('1,234.50'); + }); + + it('distinguishes an absent amount from a zero balance', () => { + expect(formatMoney(null, 'JPY', 'en-US')).toBe('—'); + expect(formatMoney(undefined, 'JPY', 'en-US')).toBe('—'); + expect(formatMoney('', 'JPY', 'en-US')).toBe('—'); + expect(formatMoney('0', 'JPY', 'en-US')).toContain('0'); + expect(formatMoney('0', 'JPY', 'en-US')).not.toBe('—'); + }); + + it('degrades to a plain number for an unknown code rather than throwing', () => { + // A bad code must not take down a page that was only showing a total. + expect(formatMoney('1000', 'NOTACODE', 'en-US')).toContain('1,000'); + }); + + it('returns an em dash for a non-numeric amount', () => { + expect(formatMoney('not money', 'JPY', 'en-US')).toBe('—'); + }); +}); diff --git a/apps/dashboard/src/lib/money.ts b/apps/dashboard/src/lib/money.ts index 26cae3c8..dcd2e54c 100644 --- a/apps/dashboard/src/lib/money.ts +++ b/apps/dashboard/src/lib/money.ts @@ -12,27 +12,21 @@ * number of fraction digits for every ISO 4217 code, so it does the work here. */ -/** Fallback when a record carries no currency and no property is selected. */ -export const DEFAULT_CURRENCY = 'USD'; - /** - * The active property's currency, pushed here by PropertyContext. + * THERE IS NO DEFAULT CURRENCY, deliberately. + * + * This file used to export DEFAULT_CURRENCY = 'USD' and fall back to it + * whenever a record carried no code — which reintroduced, one line below the + * comment explaining why it is wrong, exactly the bug it was written to fix. + * A property trading in JPY renders a real ¥151,110 balance as a dollar figure + * with two decimal places when the code is missing: a materially wrong number, + * on a live ledger, in whichever party's disfavour the reader happens to guess. * - * Same pattern the API client already uses for propertyId (`setPropertyId` in - * lib/api.ts): a module-level value the context keeps current. It means a money - * render does not need the currency threaded into every component that happens - * to display an amount — dozens of call sites across the dashboard, many inside - * helpers that cannot call a hook at all. + * A symbol we invented is worse than no symbol at all, because it looks + * authoritative. So an absent currency now renders the number PLAINLY — grouped + * but unsymbolled — which is honest about what we know and visibly odd enough + * that someone asks, rather than quietly wrong. */ -let activeCurrency = DEFAULT_CURRENCY; - -export function setActiveCurrency(code?: string | null) { - activeCurrency = (code || DEFAULT_CURRENCY).toUpperCase(); -} - -export function getActiveCurrency() { - return activeCurrency; -} /** * Format a money value for display. @@ -50,7 +44,11 @@ export function formatMoney( const value = typeof amount === 'string' ? Number(amount) : amount; if (!Number.isFinite(value)) return '—'; - const code = (currencyCode || activeCurrency).toUpperCase(); + const code = (currencyCode || '').trim().toUpperCase(); + if (!code) { + // No code, no symbol. Never guess one. + return value.toLocaleString(locale); + } try { return new Intl.NumberFormat(locale, { style: 'currency', @@ -75,7 +73,8 @@ export function formatMoneyPlain( const value = typeof amount === 'string' ? Number(amount) : amount; if (!Number.isFinite(value)) return '—'; - const code = (currencyCode || activeCurrency).toUpperCase(); + const code = (currencyCode || '').trim().toUpperCase(); + if (!code) return value.toLocaleString(locale); try { const digits = new Intl.NumberFormat(locale, { style: 'currency', diff --git a/apps/dashboard/src/pages/Accounting.tsx b/apps/dashboard/src/pages/Accounting.tsx index 3dd158f5..69c5e70a 100644 --- a/apps/dashboard/src/pages/Accounting.tsx +++ b/apps/dashboard/src/pages/Accounting.tsx @@ -1,13 +1,13 @@ import { useState } from 'react'; -import { formatMoney } from '../lib/money'; import { Routes, Route, Link } from 'react-router-dom'; import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query'; import { Calculator, Plus, Download, BarChart3, Pencil, Archive } from 'lucide-react'; import { format } from 'date-fns'; import { api } from '../lib/api'; -import { moneyString, requirePropertyId } from '../lib/api-helpers'; +import { moneyString, requirePropertyId, requireCurrency } from '../lib/api-helpers'; import { useProperty } from '../context/PropertyContext'; import Modal from '../components/ui/Modal'; +import { formatMoney } from '../lib/money'; import { useTranslation } from 'react-i18next'; interface Deposit { @@ -147,6 +147,7 @@ function AccountingHome() { const recordDeposit = useMutation({ mutationFn: () => { requirePropertyId(propertyId); + requireCurrency(currencyCode); return api.post('/v1/deposits', { propertyId, amount: moneyString(depositAmount), @@ -212,6 +213,7 @@ function AccountingHome() { const createLedger = useMutation({ mutationFn: () => { requirePropertyId(propertyId); + requireCurrency(currencyCode); return api.post('/v1/ar/ledgers', { propertyId, name: ledgerName, @@ -341,12 +343,12 @@ function AccountingHome() { {(Object.keys(AGING_LABELS) as (keyof AgingBuckets)[]).map((key) => (
    {AGING_LABELS[key]} - {formatMoney(report.buckets[key] ?? 0)} + {formatMoney(report.buckets[key] ?? 0, currencyCode)}
    ))}
    {t('accounting.total')} - {formatMoney(report.total ?? 0)} + {formatMoney(report.total ?? 0, currencyCode)}
    ) : ( @@ -397,7 +399,7 @@ function AccountingHome() {