Skip to content

fix(frontend): plot one savings series per axis with shared formatters #1385

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
IAmJSD merged 7 commits into from
Oct 24, 2026
Merged

fix(frontend): plot one savings series per axis with shared formatters #1385

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
003170d
feat(frontend): add formatCompactCurrency for chart axes
IAmJSD Oct 23, 2026
43d1d5f
fix(frontend): plot one savings series on one axis with shared format…
IAmJSD Oct 23, 2026
b8aa479
Merge remote-tracking branch 'origin/main' into ui/issue-749
IAmJSD Oct 24, 2026
7927c74
Merge remote-tracking branch 'origin/main' into ui/issue-749
IAmJSD Oct 24, 2026
c6db683
Merge remote-tracking branch 'origin/main' into ui/issue-749
IAmJSD Oct 24, 2026
33a18e5
fix(frontend): share formatCurrency's absent marker in formatCompactC…
IAmJSD Oct 24, 2026
55e4e29
Merge remote-tracking branch 'origin/main' into ui/issue-749
IAmJSD Oct 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
168 changes: 82 additions & 86 deletions frontend/src/__tests__/savings-history.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,10 @@ describe('Savings History Module', () => {
<option value="monthly" selected>Monthly</option>
<option value="yearly">Yearly</option>
</select>
<select id="savings-series">
<option value="period" selected>Savings per period</option>
<option value="cumulative">Cumulative savings</option>
</select>
<button id="refresh-savings-btn">Refresh</button>
<div id="savings-history-container">
<canvas id="savings-history-chart"></canvas>
Expand Down Expand Up @@ -332,11 +336,11 @@ describe('Savings History Module', () => {
jest.useRealTimers();
}

expect(document.getElementById('period-savings')?.textContent).toBe('$1.80K');
expect(document.getElementById('period-savings')?.textContent).toBe('$1,800.00');
// 90d window: the average spreads the authoritative total over 90 days.
expect(document.getElementById('avg-hourly-savings')?.textContent).toBe('$20.00/mo');
// Peak has no wire counterpart, so it comes from the buckets.
expect(document.getElementById('peak-savings')?.textContent).toBe('$2.00K/mo');
expect(document.getElementById('peak-savings')?.textContent).toBe('$2,000.00/mo');
});

test('ignores the never-sent legacy summary keys (issue #276)', async () => {
Expand Down Expand Up @@ -393,7 +397,7 @@ describe('Savings History Module', () => {
expect(mockChartInstance.destroy).toHaveBeenCalled();
});

test('formats currency values over 1000 with K suffix', async () => {
test('formats currency values over 1000 with the shared formatter (#749)', async () => {
const mockData = {
summary: { total_monthly_savings: 5000, total_annual_savings: 60000 },
data_points: [
Expand All @@ -405,7 +409,7 @@ describe('Savings History Module', () => {
await loadSavingsHistory();

const periodSavingsEl = document.getElementById('period-savings');
expect(periodSavingsEl?.textContent).toBe('$5.00K');
expect(periodSavingsEl?.textContent).toBe('$5,000.00');
});

test('formats currency values under 1000 without suffix', async () => {
Expand Down Expand Up @@ -476,19 +480,18 @@ describe('Savings History Module', () => {

await loadSavingsHistory();

// Issue #749: one series (period savings as bars) on one y-axis.
expect(Chart).toHaveBeenCalledWith(
expect.any(HTMLCanvasElement),
expect.objectContaining({
type: 'line',
type: 'bar',
data: expect.objectContaining({
datasets: expect.arrayContaining([
datasets: [
expect.objectContaining({
type: 'bar',
label: 'Period Savings'
}),
expect.objectContaining({
label: 'Cumulative Savings'
})
])
]
}),
options: expect.objectContaining({
responsive: true,
Expand Down Expand Up @@ -794,6 +797,7 @@ describe('Savings History Module', () => {
(getSavingsAnalytics as jest.Mock).mockResolvedValue({
data_points: manyDataPoints
});
(document.getElementById('savings-series') as HTMLSelectElement).value = 'cumulative';

await loadSavingsHistory();

Expand All @@ -812,6 +816,7 @@ describe('Savings History Module', () => {
(getSavingsAnalytics as jest.Mock).mockResolvedValue({
data_points: fewDataPoints
});
(document.getElementById('savings-series') as HTMLSelectElement).value = 'cumulative';

await loadSavingsHistory();

Expand Down Expand Up @@ -843,9 +848,11 @@ describe('Savings History Module', () => {
const chartCall = (Chart as unknown as jest.Mock).mock.calls[0];
const yAxisCallback = chartCall[1].options.scales.y.ticks.callback;

// Test with number value
expect(yAxisCallback(50)).toBe('$50.00');
// Compact shared formatter, not `$${x.toFixed(2)}` (issue #749)
expect(yAxisCallback(50)).toBe('$50');
expect(yAxisCallback(123.456)).toBe('$123.46');
expect(yAxisCallback(1500)).toBe('$1.5K');
expect(yAxisCallback(12500)).toBe('$12.5K');
});

test('y-axis tick callback formats currency for string values', async () => {
Expand All @@ -862,64 +869,9 @@ describe('Savings History Module', () => {
const yAxisCallback = chartCall[1].options.scales.y.ticks.callback;

// Test with string value
expect(yAxisCallback('75.5')).toBe('$75.50');
expect(yAxisCallback('100')).toBe('$100.00');
});

test('y1-axis tick callback formats large values with K suffix', async () => {
const mockData = {
data_points: [
{ timestamp: '2024-01-01T00:00:00Z', total_savings: 10, cumulative_savings: 10, total_upfront: 100, purchase_count: 1 }
]
};
(getSavingsAnalytics as jest.Mock).mockResolvedValue(mockData);

await loadSavingsHistory();

const chartCall = (Chart as unknown as jest.Mock).mock.calls[0];
const y1AxisCallback = chartCall[1].options.scales.y1.ticks.callback;

// Test with large number values (>= 1000)
expect(y1AxisCallback(1000)).toBe('$1.0K');
expect(y1AxisCallback(5000)).toBe('$5.0K');
expect(y1AxisCallback(12500)).toBe('$12.5K');
});

test('y1-axis tick callback formats small values without K suffix', async () => {
const mockData = {
data_points: [
{ timestamp: '2024-01-01T00:00:00Z', total_savings: 10, cumulative_savings: 10, total_upfront: 100, purchase_count: 1 }
]
};
(getSavingsAnalytics as jest.Mock).mockResolvedValue(mockData);

await loadSavingsHistory();

const chartCall = (Chart as unknown as jest.Mock).mock.calls[0];
const y1AxisCallback = chartCall[1].options.scales.y1.ticks.callback;

// Test with small number values (< 1000)
expect(y1AxisCallback(50)).toBe('$50');
expect(y1AxisCallback(999)).toBe('$999');
expect(y1AxisCallback(0)).toBe('$0');
});

test('y1-axis tick callback handles string values', async () => {
const mockData = {
data_points: [
{ timestamp: '2024-01-01T00:00:00Z', total_savings: 10, cumulative_savings: 10, total_upfront: 100, purchase_count: 1 }
]
};
(getSavingsAnalytics as jest.Mock).mockResolvedValue(mockData);

await loadSavingsHistory();

const chartCall = (Chart as unknown as jest.Mock).mock.calls[0];
const y1AxisCallback = chartCall[1].options.scales.y1.ticks.callback;

// Test with string values
expect(y1AxisCallback('2500')).toBe('$2.5K');
expect(y1AxisCallback('500')).toBe('$500');
expect(yAxisCallback('75.5')).toBe('$75.5');
expect(yAxisCallback('100')).toBe('$100');
expect(yAxisCallback('2500')).toBe('$2.5K');
});

test('tooltip label callback formats period savings with unit suffix (default monthly)', async () => {
Expand Down Expand Up @@ -951,19 +903,19 @@ describe('Savings History Module', () => {
]
};
(getSavingsAnalytics as jest.Mock).mockResolvedValue(mockData);
(document.getElementById('savings-series') as HTMLSelectElement).value = 'cumulative';

await loadSavingsHistory();

const chartCall = (Chart as unknown as jest.Mock).mock.calls[0];
const tooltipLabelCallback = chartCall[1].options.plugins.tooltip.callbacks.label;

// Test cumulative savings (datasetIndex 1)
const cumulativeContext = {
raw: 1234.56,
datasetIndex: 1,
datasetIndex: 0,
dataset: { label: 'Cumulative Savings' }
};
expect(tooltipLabelCallback(cumulativeContext)).toBe('Cumulative Savings: $1234.56');
expect(tooltipLabelCallback(cumulativeContext)).toBe('Cumulative Savings: $1,234.56');
});

test('tooltip label callback handles null/undefined raw value', async () => {
Expand All @@ -989,10 +941,10 @@ describe('Savings History Module', () => {

const undefinedContext = {
raw: undefined,
datasetIndex: 1,
dataset: { label: 'Cumulative Savings' }
datasetIndex: 0,
dataset: { label: 'Period Savings' }
};
expect(tooltipLabelCallback(undefinedContext)).toBe('Cumulative Savings: $0.00');
expect(tooltipLabelCallback(undefinedContext)).toBe('Period Savings: $0.00/mo');
});

// Regression tests for QA 2.2 / 2.3 / 2.4 (do not remove)
Expand Down Expand Up @@ -1060,7 +1012,7 @@ describe('Savings History Module', () => {
expect(chartCall[1].data.labels).toBeUndefined();
});

test('QA 2.4: y and y1 axes both have maxTicksLimit to prevent tick instability', async () => {
test('QA 2.4: the y-axis has maxTicksLimit to prevent tick instability', async () => {
const mockData = {
data_points: [
{ timestamp: '2024-01-01T00:00:00Z', total_savings: 10, cumulative_savings: 10, total_upfront: 100, purchase_count: 1 }
Expand All @@ -1072,15 +1024,12 @@ describe('Savings History Module', () => {

const chartCall = (Chart as unknown as jest.Mock).mock.calls[0];
const yTicks = chartCall[1].options.scales.y.ticks;
const y1Ticks = chartCall[1].options.scales.y1.ticks;

expect(typeof yTicks.maxTicksLimit).toBe('number');
expect(yTicks.maxTicksLimit).toBeGreaterThan(0);
expect(typeof y1Ticks.maxTicksLimit).toBe('number');
expect(y1Ticks.maxTicksLimit).toBeGreaterThan(0);
});

test('QA 2.5: y1-axis formatter does not collapse distinct floats to the same integer label', async () => {
test('QA 2.5: the y-axis formatter does not collapse distinct floats to the same integer label', async () => {
const mockData = {
data_points: [
{ timestamp: '2024-01-01T00:00:00Z', total_savings: 10, cumulative_savings: 10, total_upfront: 100, purchase_count: 1 }
Expand All @@ -1091,7 +1040,7 @@ describe('Savings History Module', () => {
await loadSavingsHistory();

const chartCall = (Chart as unknown as jest.Mock).mock.calls[0];
const y1Cb = chartCall[1].options.scales.y1.ticks.callback;
const y1Cb = chartCall[1].options.scales.y.ticks.callback;

// Two floats that differ in decimal but share the same integer part must
// produce distinct labels (QA 2.5).
Expand Down Expand Up @@ -1452,8 +1401,8 @@ describe('Savings History Module', () => {

const avgEl = document.getElementById('avg-hourly-savings');
expect(avgEl?.textContent).toContain('/yr');
// 730 * 12 = $8760.00 -> formatted as $8.76K
expect(avgEl?.textContent).toContain('$8.76K');
// 730 * 12 = $8,760.00
expect(avgEl?.textContent).toContain('$8,760.00');
});

test('avg-savings-label heading changes to reflect selected unit', async () => {
Expand Down Expand Up @@ -1518,10 +1467,10 @@ describe('Savings History Module', () => {
expect(document.getElementById('period-savings')?.textContent).toBe('$1.00');
expect(document.getElementById('period-savings-unit')?.textContent).toContain('hourly');

// Yearly: 730 * 12 = 8760 -> $8.76K; sub-line updates to 'yearly'
// Yearly: 730 * 12 = 8760 -> $8,760.00; sub-line updates to 'yearly'
unitSelect.value = 'yearly';
await loadSavingsHistory();
expect(document.getElementById('period-savings')?.textContent).toBe('$8.76K');
expect(document.getElementById('period-savings')?.textContent).toBe('$8,760.00');
expect(document.getElementById('period-savings-unit')?.textContent).toContain('yearly');
});

Expand All @@ -1537,4 +1486,51 @@ describe('Savings History Module', () => {
expect(tooltipCb(ctx)).toContain('/hr');
});
});

describe('one series on one axis (issue #749)', () => {
const mockData = {
data_points: [
{ timestamp: '2024-01-01T00:00:00Z', total_savings: 100, cumulative_savings: 100, total_upfront: 0, purchase_count: 1 },
{ timestamp: '2024-01-02T00:00:00Z', total_savings: 300, cumulative_savings: 400, total_upfront: 0, purchase_count: 1 },
],
};

test('period view draws bars on a single y-axis', async () => {
(getSavingsAnalytics as jest.Mock).mockResolvedValue(mockData);
await loadSavingsHistory();
const config = (Chart as unknown as jest.Mock).mock.calls[0][1];
expect(Object.keys(config.options.scales)).toEqual(['x', 'y']);
expect(config.data.datasets).toHaveLength(1);
expect(config.data.datasets[0].type).toBe('bar');
expect(config.data.datasets[0].data.map((d: { y: number }) => d.y)).toEqual([100, 300]);
expect(config.options.scales.y.title.text).toBe('Savings per period (Monthly)');
});

test('cumulative view plots the running total alone, without a rate suffix', async () => {
(getSavingsAnalytics as jest.Mock).mockResolvedValue(mockData);
(document.getElementById('savings-series') as HTMLSelectElement).value = 'cumulative';
await loadSavingsHistory();
const config = (Chart as unknown as jest.Mock).mock.calls[0][1];
expect(Object.keys(config.options.scales)).toEqual(['x', 'y']);
expect(config.type).toBe('line');
expect(config.data.datasets).toHaveLength(1);
expect(config.data.datasets[0].label).toBe('Cumulative Savings');
expect(config.data.datasets[0].data.map((d: { y: number }) => d.y)).toEqual([100, 400]);
expect(config.options.scales.y.title.text).toBe('Cumulative savings');
const tooltip = config.options.plugins.tooltip.callbacks.label;
expect(tooltip({ raw: { x: 0, y: 400 }, dataset: { label: 'Cumulative Savings' } })).toBe('Cumulative Savings: $400.00');
});

test('changing the series reloads the chart once, even after a second init', async () => {
(getSavingsAnalytics as jest.Mock).mockResolvedValue(mockData);
initSavingsHistory();
initSavingsHistory();
(getSavingsAnalytics as jest.Mock).mockClear();
const series = document.getElementById('savings-series') as HTMLSelectElement;
series.value = 'cumulative';
series.dispatchEvent(new Event('change'));
await new Promise(resolve => setTimeout(resolve, 50));
expect(getSavingsAnalytics).toHaveBeenCalledTimes(1);
});
});
});
21 changes: 21 additions & 0 deletions frontend/src/__tests__/utils.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
* Unit tests for utility functions
*/
import {
formatCompactCurrency,
formatCurrency,
formatDate,
formatDateTime,
Expand Down Expand Up @@ -640,6 +641,26 @@ describe('jsonClone (explicit JSON-serialisation variant)', () => {
// ---------------------------------------------------------------------------
// Regression tests for finding 11-N2: formatCurrency distinguishes absent from $0.
// ---------------------------------------------------------------------------
describe('formatCompactCurrency (issue #749)', () => {
test('abbreviates thousands and millions like a chart axis', () => {
expect(formatCompactCurrency(0)).toBe('$0');
expect(formatCompactCurrency(950)).toBe('$950');
expect(formatCompactCurrency(1500)).toBe('$1.5K');
expect(formatCompactCurrency(12345)).toBe('$12.35K');
expect(formatCompactCurrency(1234567)).toBe('$1.23M');
expect(formatCompactCurrency(12345, 1)).toBe('$12.3K');
});

test('keeps close fractional ticks distinct', () => {
expect(formatCompactCurrency(0.1)).not.toBe(formatCompactCurrency(0.2));
});

test('uses the same absent marker as formatCurrency', () => {
expect(formatCompactCurrency(null)).toBe(formatCurrency(null));
expect(formatCompactCurrency(NaN)).toBe(formatCurrency(NaN));
});
});

describe('formatCurrency (11-N2: absent vs real zero)', () => {
test('null returns an em dash, not $0', () => {
expect(formatCurrency(null as unknown as number)).toBe('—');
Expand Down
6 changes: 6 additions & 0 deletions frontend/src/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,12 @@ <h2>Savings History</h2>
<option value="yearly">Yearly</option>
</select>
</label>
<label>Show:
<select id="savings-series">
<option value="period" selected>Savings per period</option>
<option value="cumulative">Cumulative savings</option>
</select>
</label>
<button id="refresh-savings-btn" class="btn-small">Refresh</button>
</div>
</div>
Expand Down
Loading
Loading