From 31bc66a6297aca9a6b21694a14e691b8183537e9 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 5 Oct 2026 18:52:04 +0200 Subject: [PATCH] fix(frontend/settings): keep a stored default_coverage of 0 on load loadGlobalSettings read default_coverage with `|| 80`, so a stored 0 rendered as 80 in the Target coverage input and in the cached global default behind the override modal's "Inherit (currently: X%)" label. An unedited Save then PUT default_coverage 80, and since #500 the changed global also propagates 80 to every service config's coverage. Use `?? 80` at both sites so only an absent value falls back to 80, matching the per-service coverage inputs. The backend accepts 0-100. Closes #135 --- frontend/src/__tests__/settings.test.ts | 84 +++++++++++++++++++++++++ frontend/src/settings.ts | 4 +- 2 files changed, 86 insertions(+), 2 deletions(-) diff --git a/frontend/src/__tests__/settings.test.ts b/frontend/src/__tests__/settings.test.ts index 0d74dfaa9..519642150 100644 --- a/frontend/src/__tests__/settings.test.ts +++ b/frontend/src/__tests__/settings.test.ts @@ -8,6 +8,7 @@ import { setupSettingsHandlers, copyToClipboard, isUnsavedChanges, + openOverrideModal, } from '../settings'; // Mock the api module @@ -1556,4 +1557,87 @@ describe('Settings Module', () => { expect(isUnsavedChanges()).toBe(true); }); }); + + // Issue #135: `default_coverage || 80` rewrote a stored 0 to 80, and the + // next Save persisted 80 without the user touching the field. + describe('default_coverage 0 round-trips through load and save (issue #135)', () => { + const configWithCoverage = (global: Record, services: unknown[] = []) => ({ + global: { + enabled_providers: ['aws'], + notification_email: '', + auto_collect: true, + default_term: 3, + default_payment: 'all-upfront', + notification_days_before: 3, + ...global, + }, + services, + credentials: {}, + }); + + const serviceCall = (service: string) => + (api.updateServiceConfig as jest.Mock).mock.calls.find(([p, s]) => p === 'aws' && s === service); + + const saveUnchanged = async () => { + (api.updateConfig as jest.Mock).mockResolvedValue({}); + (api.updateServiceConfig as jest.Mock).mockClear().mockResolvedValue(undefined); + await saveGlobalSettings({ preventDefault: jest.fn() } as unknown as Event); + }; + + test('a stored 0 renders as 0 and an unedited Save sends 0, not 80', async () => { + (api.getConfig as jest.Mock).mockResolvedValue(configWithCoverage({ default_coverage: 0 })); + await loadGlobalSettings(); + + expect((document.getElementById('setting-default-coverage') as HTMLInputElement).value).toBe('0'); + expect((document.getElementById('aws-savings-plans-compute-coverage') as HTMLInputElement).value).toBe('0'); + + await saveUnchanged(); + + expect((api.updateConfig as jest.Mock).mock.calls[0]![0].default_coverage).toBe(0); + expect(serviceCall('savings-plans-compute')![2].coverage).toBe(0); + expect(serviceCall('ec2')![2].coverage).toBe(0); + }); + + test('an absent default_coverage still renders the 80 default', async () => { + (api.getConfig as jest.Mock).mockResolvedValue(configWithCoverage({})); + await loadGlobalSettings(); + + expect((document.getElementById('setting-default-coverage') as HTMLInputElement).value).toBe('80'); + + await saveUnchanged(); + + expect((api.updateConfig as jest.Mock).mock.calls[0]![0].default_coverage).toBe(80); + }); + + test('a per-service coverage of 0 round-trips under a non-zero global', async () => { + (api.getConfig as jest.Mock).mockResolvedValue(configWithCoverage( + { default_coverage: 50 }, + [{ provider: 'aws', service: 'savings-plans-compute', enabled: true, term: 3, payment: 'all-upfront', coverage: 0 }], + )); + await loadGlobalSettings(); + + expect((document.getElementById('aws-savings-plans-compute-coverage') as HTMLInputElement).value).toBe('0'); + expect((document.getElementById('aws-savings-plans-ec2instance-coverage') as HTMLInputElement).value).toBe('50'); + + await saveUnchanged(); + + expect(serviceCall('savings-plans-compute')![2].coverage).toBe(0); + expect(serviceCall('savings-plans-ec2instance')![2].coverage).toBe(50); + }); + + test('the override modal inherit label shows the stored 0%', async () => { + const modal = document.createElement('div'); + modal.id = 'override-modal'; + const covInput = document.createElement('input'); + covInput.id = 'override-coverage'; + modal.appendChild(covInput); + document.body.appendChild(modal); + + (api.getConfig as jest.Mock).mockResolvedValue(configWithCoverage({ default_coverage: 0 })); + await loadGlobalSettings(); + openOverrideModal('acc-1', 'aws', [], document.createElement('div')); + + expect(covInput.placeholder).toBe('Inherit (currently: 0%)'); + }); + }); }); diff --git a/frontend/src/settings.ts b/frontend/src/settings.ts index 7b3fb9d18..6c48b5e5f 100644 --- a/frontend/src/settings.ts +++ b/frontend/src/settings.ts @@ -3181,7 +3181,7 @@ export async function loadGlobalSettings(): Promise { if (paymentSelect) paymentSelect.value = data.global.default_payment || 'all-upfront'; const coverageInput = document.getElementById('setting-default-coverage') as HTMLInputElement | null; - if (coverageInput) coverageInput.value = String(data.global.default_coverage || 80); + if (coverageInput) coverageInput.value = String(data.global.default_coverage ?? 80); // Cache defaults for the override modal "Inherit (currently: X)" labels // (issue #112). Must be updated here so reopening the override modal @@ -3189,7 +3189,7 @@ export async function loadGlobalSettings(): Promise { cachedGlobalDefaults = { term: data.global.default_term || 3, payment: data.global.default_payment || 'all-upfront', - coverage: data.global.default_coverage || 80, + coverage: data.global.default_coverage ?? 80, }; const notifyDaysInput = document.getElementById('setting-notification-days') as HTMLInputElement | null;