From 788e80588df967f00ca75eb7d869bb7a431ce8de Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Sat, 8 Aug 2026 01:23:14 +0200 Subject: [PATCH 1/3] fix(frontend): pin formatCurrency to en-US instead of the host locale MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit formatCurrency() passed undefined as the locale to toLocaleString, so its digit grouping followed whichever locale the runtime (browser or, for tests, the machine running jest) happened to default to. On a locale that uses '.' as the thousands separator, $1,000 rendered as $1.000. Eight tests across utils.test.ts, riexchange.test.ts and approval-details.test.ts asserted the en-US grouping directly, so they failed locally on such a machine and passed in CI purely by accident of environment -- a genuine formatting regression under CI's own locale would have stayed green too. Pins the locale to 'en-US', matching the convention formatDate() and formatDateTime() already use in this same file for the identical ambiguity reason. formatCurrency's currency-symbol prefix is already locale-invariant (a fixed "$"/"€"/etc, not Intl currency-style formatting), so the digits after it should be too: CUDly's money model is USD-only end to end, and two admins should see the same digits for the same dollar figure regardless of their browser's locale. This is a production behavior change for real users on a non-US-locale browser, not only a test fix. Verified the 8 tests fail under LANG=de_DE.UTF-8 and pass under LANG=en_US.UTF-8 before this change, and pass under both locales after. Full suite (2781 tests) also passes under both locales post-fix. dashboard.ts has several independent toLocaleString() calls in Chart.js tooltip/axis-tick callbacks with the same unpinned-locale pattern; left untouched since no test currently exercises them and they are out of this issue's scope. --- frontend/src/utils.ts | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/frontend/src/utils.ts b/frontend/src/utils.ts index ecc27c164..29f713142 100644 --- a/frontend/src/utils.ts +++ b/frontend/src/utils.ts @@ -21,6 +21,18 @@ export const CURRENCY_DEFAULT_DIGITS = 0; * Purchase History summary cards and RI Exchange cost chips, pass `digits: * 2`. Having a single helper keeps "$0" / "$0.00" / "$0.00/hr" from * diverging across the app. + * + * The digit grouping is pinned to en-US rather than following the host + * locale (issue #1728), mirroring formatDate/formatDateTime below: the + * `currency` prefix is already a fixed symbol regardless of locale, so the + * number after it should be unambiguous too. Two admins viewing the same + * dollar figure on browsers set to different locales must see the same + * digits and separators -- support screenshots and numbers read aloud on a + * call need to match across machines, and CUDly's money model is USD-only + * end to end. Letting `toLocaleString` fall back to the host locale also + * made every consumer of this helper an accidental host-locale test, which + * is why 8 tests across 3 files failed only on developer machines whose + * locale uses `.` as the thousands separator while passing in CI. */ export function formatCurrency( value: number | null | undefined, @@ -32,7 +44,7 @@ export function formatCurrency( if (value === null || value === undefined || !Number.isFinite(value)) { return '--'; } - return `${currency}${value.toLocaleString(undefined, { + return `${currency}${value.toLocaleString('en-US', { minimumFractionDigits: digits, maximumFractionDigits: digits })}`; From 5529dd65c97ac529b842574f39e7ee9ed20c9140 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Sat, 8 Aug 2026 01:31:42 +0200 Subject: [PATCH 2/3] fix(frontend): route dashboard chart tooltips through formatCurrency dashboard.ts's Chart.js tooltip and axis-tick callbacks built dollar strings by hand with `$${x.toLocaleString()}`, six occurrences across two charts. Same unpinned-locale defect as formatCurrency() before the previous commit -- these fell back to the host locale too -- but the deeper issue was duplication: dashboard.ts already imports formatCurrency from utils.ts and uses it elsewhere in the same file, so these six were reinventing it inline instead of calling it. Routes all six through the shared helper. Mechanical: each site formats a guaranteed-finite number (never null/undefined) with no fraction digits and a literal '$' prefix, exactly formatCurrency's default signature, so the change is a straight substitution with no behavior difference beyond the locale fix itself. These call sites render into a Chart.js , not the DOM, so no existing test exercises them and none are being added here -- the change is verified by tsc, eslint, and the full suite passing under both locales, not by new coverage of previously-untested chart code. --- frontend/src/dashboard.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/frontend/src/dashboard.ts b/frontend/src/dashboard.ts index 9f76f89d4..fc7cba095 100644 --- a/frontend/src/dashboard.ts +++ b/frontend/src/dashboard.ts @@ -962,7 +962,7 @@ export function renderSavingsByService( // tooltip; the range datasets share the full breakdown. if (ctx.dataset?.label === 'Current / Committed') { const current = byService[svc]?.current_savings ?? 0; - return `Current / Committed: $${current.toLocaleString()}`; + return `Current / Committed: ${formatCurrency(current)}`; } const current = byService[svc]?.current_savings ?? 0; const maxRec = s?.max ?? 0; @@ -973,10 +973,10 @@ export function renderSavingsByService( const pct = totalSavings > 0 ? ((total / totalSavings) * 100).toFixed(1) : '0.0'; const lines = [ `Service: ${svc}`, - `Total: $${total.toLocaleString()} (${pct}% of all services)`, - `Current / Committed: $${current.toLocaleString()}`, - `Lowest option: $${lowestOption.toLocaleString()}`, - `Upside: $${upside.toLocaleString()}`, + `Total: ${formatCurrency(total)} (${pct}% of all services)`, + `Current / Committed: ${formatCurrency(current)}`, + `Lowest option: ${formatCurrency(lowestOption)}`, + `Upside: ${formatCurrency(upside)}`, ]; if (s?.minLabel) lines.push(`Min option: ${s.minLabel}`); if (s?.maxLabel) lines.push(`Max option: ${s.maxLabel}`); @@ -994,7 +994,7 @@ export function renderSavingsByService( stacked: true, beginAtZero: true, title: { display: true, text: 'Monthly savings ($)' }, - ticks: { callback: (v) => '$' + (v as number).toLocaleString() }, + ticks: { callback: (v) => formatCurrency(v as number) }, }, }, }, @@ -1131,7 +1131,7 @@ export async function loadSavingsTrendChart(): Promise { const raw = items[0]?.raw as { x: number; y: number } | undefined; return raw?.x != null ? formatTrendAxisTick(raw.x, interval) : ''; }, - label: (ctx) => `Cumulative savings: $${((ctx.raw as { x: number; y: number }).y).toLocaleString()}`, + label: (ctx) => `Cumulative savings: ${formatCurrency((ctx.raw as { x: number; y: number }).y)}`, }, }, }, @@ -1147,7 +1147,7 @@ export async function loadSavingsTrendChart(): Promise { }, y: { beginAtZero: true, - ticks: { callback: (v) => '$' + (v as number).toLocaleString() }, + ticks: { callback: (v) => formatCurrency(v as number) }, }, }, }, From 7cb7f3d87a09e5a34cafcb7bf020d81b98fd73f6 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Sat, 8 Aug 2026 04:22:00 +0200 Subject: [PATCH 3/3] fix(frontend): pin getDateParts month abbreviation to en-US too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit getDateParts() called toLocaleString('default', {month: 'short'}) -- an explicit request for the host locale, the exact thing PR #1732 exists to eliminate from formatCurrency/formatDate/formatDateTime. dashboard.ts imports both formatCurrency and getDateParts and renders them side by side on the upcoming-purchases cards, so the dashboard was showing en-US money and en-US dates except one month abbreviation that still followed the browser (e.g. "Mär" under de-DE while everything else read "Mar"). Pins it to 'en-US', matching the other helpers in this file. Also adds regression coverage the original pin lacked: the pre-PR suite passed 91/91 under en-US (CI's own locale), so nothing actually exercised the locale-dependent branch. utils.test.ts now simulates a de-DE host default (by intercepting the "no explicit locale" call shape used by unpinned toLocaleString calls) and asserts formatCurrency and getDateParts still render en-US output. Verified both assertions fail pre-fix and pass post-fix: - reverting getDateParts to 'default': expected "Mar", got "Mär" - reverting formatCurrency to toLocaleString(undefined, ...): expected "$1,000", got "$1.000" Corrects two now-stale comments in recommendations.test.ts that described formatCurrency as locale-dependent (toLocaleString(undefined, ...)); it's been pinned to en-US since #1732. Filed #1747 for a separate, out-of-scope duplication issue found in review: modules/savings-history.ts has its own private formatCurrency shadowing this one (different, toFixed-based implementation, not locale-dependent). --- .../src/__tests__/recommendations.test.ts | 14 ++--- frontend/src/__tests__/utils.test.ts | 52 ++++++++++++++++++- frontend/src/utils.ts | 7 ++- 3 files changed, 63 insertions(+), 10 deletions(-) diff --git a/frontend/src/__tests__/recommendations.test.ts b/frontend/src/__tests__/recommendations.test.ts index 9c6be4c35..7f1215014 100644 --- a/frontend/src/__tests__/recommendations.test.ts +++ b/frontend/src/__tests__/recommendations.test.ts @@ -1218,10 +1218,10 @@ describe('Recommendations Module', () => { await loadRecommendations(); const summary = document.getElementById('recommendations-action-summary'); - // #281: min!=max so formatSavingsRange emits the "X – Y" form. The - // commas in $1,100 / $1,800 depend on the runtime's default locale - // (formatCurrency uses toLocaleString(undefined, ...)); JSDOM may emit - // "$1100" without a separator, so the regex makes the comma optional. + // #281: min!=max so formatSavingsRange emits the "X – Y" form. + // formatCurrency is pinned to en-US (#1728), so $1,100 / $1,800 always + // render with a comma; the regex still leaves it optional so the + // assertion doesn't couple to that separator choice. expect(summary?.textContent).toMatch(/\$300\s*[–\-]\s*\$500\/mo/); expect(summary?.textContent).toMatch(/\$1,?100\s*[–\-]\s*\$1,?800 upfront/); expect(summary?.textContent).toMatch(/2 cells\b/); @@ -7325,9 +7325,9 @@ describe('Issue #484: numeric filter matches the displayed rounded value', () => // changes. describe('displayPrecision agrees with formatCurrency for currency columns', () => { function fractionDigitsOf(s: string): number { - // Strip leading currency symbol(s) and any locale group separators, - // then count digits after a decimal point. Returns 0 for "$123", - // 2 for "$123.45", etc. + // Strip the leading currency symbol and formatCurrency's en-US comma + // group separators (#1728), then count digits after a decimal point. + // Returns 0 for "$123", 2 for "$123.45", etc. const stripped = s.replace(/[^0-9.]/g, ''); const dot = stripped.indexOf('.'); return dot < 0 ? 0 : stripped.length - dot - 1; diff --git a/frontend/src/__tests__/utils.test.ts b/frontend/src/__tests__/utils.test.ts index 3c5f4418e..58b4759c9 100644 --- a/frontend/src/__tests__/utils.test.ts +++ b/frontend/src/__tests__/utils.test.ts @@ -137,7 +137,9 @@ describe('getDateParts', () => { test('returns day and month for valid date', () => { const result = getDateParts('2024-03-15'); expect(result.day).toBe(15); - expect(result.month).toBeTruthy(); + // Pinned to en-US (#1728): assert the literal abbreviation, not just + // truthiness, so a locale regression here fails this test directly. + expect(result.month).toBe('Mar'); }); test('returns zeros for null/undefined', () => { @@ -159,6 +161,54 @@ describe('getDateParts', () => { }); }); +describe('locale independence (issue #1728)', () => { + // formatCurrency and getDateParts must render the same digits/month + // abbreviation no matter what locale the host browser (or, for tests, + // the machine running jest) defaults to. The pre-fix code asked for the + // host default explicitly -- `toLocaleString(undefined, ...)` in + // formatCurrency, `toLocaleString('default', ...)` in getDateParts -- + // so a real regression only shows up when that default isn't en-US. + // Rather than depend on the CI runner's own locale (which is en-US, so + // these tests would stay green with the bug present -- see PR #1732), + // simulate a de-DE host default by intercepting the "no explicit + // locale" call shape and rerouting it to a real de-DE formatter, while + // leaving the fixed code's explicit 'en-US' calls untouched. + const realNumberToLocaleString = Number.prototype.toLocaleString; + const realDateToLocaleString = Date.prototype.toLocaleString; + + beforeEach(() => { + jest.spyOn(Number.prototype, 'toLocaleString').mockImplementation( + function (this: number, locale?: Intl.LocalesArgument, options?: Intl.NumberFormatOptions) { + const hostDefault = locale === undefined ? 'de-DE' : locale; + return realNumberToLocaleString.call(this, hostDefault, options); + } + ); + jest.spyOn(Date.prototype, 'toLocaleString').mockImplementation( + function (this: Date, locale?: Intl.LocalesArgument, options?: Intl.DateTimeFormatOptions) { + const hostDefault = locale === undefined || locale === 'default' ? 'de-DE' : locale; + return realDateToLocaleString.call(this, hostDefault, options); + } + ); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + test('formatCurrency keeps en-US comma grouping under a de-DE host default', () => { + // de-DE would render this "1.000" (period as thousands separator); + // confirms formatCurrency's explicit 'en-US' argument, not the host + // default, drives the output. + expect(formatCurrency(1000)).toBe('$1,000'); + }); + + test('getDateParts keeps the en-US month abbreviation under a de-DE host default', () => { + // de-DE would render March as "Mär"; confirms getDateParts passes an + // explicit 'en-US' locale rather than 'default'. + expect(getDateParts('2024-03-15').month).toBe('Mar'); + }); +}); + describe('debounce', () => { beforeEach(() => { jest.useFakeTimers(); diff --git a/frontend/src/utils.ts b/frontend/src/utils.ts index 29f713142..66cc4bde7 100644 --- a/frontend/src/utils.ts +++ b/frontend/src/utils.ts @@ -156,7 +156,10 @@ export interface DateParts { } /** - * Get day and month from date + * Get day and month from date. The month abbreviation is pinned to en-US, + * same as formatDate/formatDateTime/formatCurrency above (issue #1728): + * 'default' asks toLocaleString for the host locale explicitly, which is + * exactly what this file's other helpers were fixed to stop doing. */ export function getDateParts(date: string | Date | null | undefined): DateParts { if (!date) return { day: 0, month: '' }; @@ -164,7 +167,7 @@ export function getDateParts(date: string | Date | null | undefined): DateParts if (isNaN(d.getTime())) return { day: 0, month: '' }; return { day: d.getDate(), - month: d.toLocaleString('default', { month: 'short' }) + month: d.toLocaleString('en-US', { month: 'short' }) }; }