fix(dashboard): there is no default currency - #327
Open
modernitconsultants wants to merge 1 commit into
Open
Conversation
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 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #301, which fixed the formatting and left the fallback.
lib/money.tsexplains, correctly and at length, why a hardcoded currency symbol is wrong for a property trading in anything but dollars — and then exportsDEFAULT_CURRENCY = 'USD'two lines later and falls back to it whenever a record carries no code. The comment describing the bug sits directly above the bug.The consequence isn't 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. That's honest about what's known and odd enough that someone asks. A symbol we invented is worse than no symbol precisely because it looks decided.
PropertyContextreturnsnullrather than substituting, including in portfolio mode, where properties may genuinely differ and null is the answer.The more serious half, which removing the default surfaced
Six pages POST the context currency into request bodies. 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 existingrequirePropertyIdidiom. Applied only to mutations that actually send a currency: guarding every mutation beside arequirePropertyIdwould block unrelated actions like a room move, and the bulk-import parser has its own localcurrencyCodefrom the pasted rows.Specs assert behaviour, not glyphs
ICU renders JPY as
¥in some versions andJP¥in others. Pinning the symbol would fail on a runner upgrade while saying nothing about the defect. What's pinned: a missing code never yields a dollar sign, JPY carries no decimal part, an absent amount is an em dash and a zero balance is not, and a bad code degrades to a number instead of throwing a page away.