fix(clients): locale-aware thousands/decimal separator parsing in MoneyInput - #14190
Closed
detail-app[bot] wants to merge 1 commit into
Closed
detail-app[bot] wants to merge 1 commit into
detail-app[bot] wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This branch was successfully deployed
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
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.
Detail bug report: View on Detail
Summary
Related Issue: polarsource/feedback#456
MoneyInputmisparsed US thousands separators (commas) as decimal separators foren-locale users, collapsing whole amounts ~1000× (e.g.5,000→5.00, submitting 500 cents instead of 500000). This affected buyer-facing Pay-What-You-Want checkout and every seller-facing pricing form, with no client or server guard catching the magnitude error.What
@polar-sh/currency:getLocaleDecimalSeparator(locale)— returns the locale's decimal separator (.or,) viaIntl.NumberFormat.parseMoneyValue(input, decimalSeparator)— locale-aware parser that normalizes a typed/pasted string to a.-decimal display value, stripping thousands separators and rounding to 2 fractional digits.onChangeparser in bothMoneyInputimplementations (packages/checkoutandpackages/ui) withparseMoneyValue, and added an optionallocaleprop (default'en') so parsing branches on the locale's decimal separator.localeprop fromCheckoutPWYWForminto itsMoneyInput.onKeyDownmulti-separator guard locale-aware.CheckoutPWYWForm.test.tsx.Why
The previous
onChangerancleaned.match(/([.,])([0-9]+)$)against the un-stripped input, so any trailing separator+digits was unconditionally treated as a decimal — regardless of locale. Forenusers,5,000matched,000and was rewritten to5.00. Thelocaleprop was only used for placeholder formatting, never for parsing. A correct fix must branch on locale: commas are thousands separators foren/ja/ko, but decimal separators forde/fr/etc. The deduped shared parser fixes bothMoneyInputcopies (checkout +@polar-sh/ui) in one place; the web-dashboard call sites gain theenfix with no call-site changes sincelocaledefaults to'en'.How
toFixeddisplay format of committed values) is also treated as a decimal so editing round-trips correctly. For period-decimal locales, a lone comma is always a thousands separator — the reported bug. A trailing separator with no fractional digits (e.g.5.) is preserved during typing and stripped on blur. At most one decimal separator is honored (the last); earlier occurrences are stripped from the integer part.MoneyInputimplementations computedecimalSeparator = getLocaleDecimalSeparator(locale)and callparseMoneyValue(input, decimalSeparator)inonChange. ThegetUnits(minor-units conversion) andonBlurhandlers are unchanged.Testing
@polar-sh/currency): 73 tests coveringgetLocaleDecimalSeparatorfor all 14 supported locales andparseMoneyValueacross the period-decimal and comma-decimal matrices (thousands stripping, mixed thousands+decimal, trailing-separator typing, 2-digit rounding, leading-zero guard, junk stripping, empty input). All pass.CheckoutPWYWForm.test.tsx): 20 tests — the 9 existing baseline cases plus en thousands-separator regression cases (5,000→500000,1,234→123400,12,345→1234500,1,000,000→100000000), en mixed (5,000.99→500099), en period rounding (5.55→555), de comma-decimal cases (12,5→1250,12,50→1250,12,500→1250,1.234,56→123456), and a regression case verifying1,234no longer surfaces a misleading "below minimum" error. All pass.@polar-sh/currency73/73 and@polar-sh/checkout451/451 (34 files). All pass.currency,checkout,ui, andweb(tsc --noEmit,oxfmt --check,oxlint .).verifier-webskill) could not be run: the in-container web frontend hit a host cgroup v2 runtime error, the hostnext devprocess doesn't survive the shell session, and Playwright is not installed in this environment. The infra + API stack was brought up and confirmed serving (/healthz→{"status":"ok"}). The behavioral coverage rests on the unit and integration suites, which verify the exact minor units reaching the API through the form's debounce path for bothenanddelocales.Checklist
Automatic Fixes PRs can be configured here.