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/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) }, }, }, }, diff --git a/frontend/src/utils.ts b/frontend/src/utils.ts index ecc27c164..66cc4bde7 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 })}`; @@ -144,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: '' }; @@ -152,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' }) }; }