diff --git a/frontend/src/__tests__/settings.test.ts b/frontend/src/__tests__/settings.test.ts index 36710802..1b33a463 100644 --- a/frontend/src/__tests__/settings.test.ts +++ b/frontend/src/__tests__/settings.test.ts @@ -655,10 +655,11 @@ describe('Settings Module', () => { window.alert = jest.fn(); const event = { preventDefault: jest.fn() } as unknown as Event; - await saveGlobalSettings(event); + await expect(saveGlobalSettings(event)).resolves.toBeUndefined(); - // Should still call updateConfig with default values - expect(api.updateConfig).toHaveBeenCalled(); + // No provider checkbox exists, so the empty list is refused (#1531) + // instead of being written. + expect(api.updateConfig).not.toHaveBeenCalled(); }); // Issue #22 follow-up: AWS Savings Plans split into four per-plan-type @@ -2144,7 +2145,7 @@ describe('Settings Module', () => { }); describe('global PUT against the real index.html (issue #539)', () => { - const loadRealPage = async (): Promise => { + const loadRealPage = async (providers: string[] = ['aws']): Promise => { // eslint-disable-next-line @typescript-eslint/no-require-imports const fs = require('fs') as typeof import('fs'); // eslint-disable-next-line @typescript-eslint/no-require-imports @@ -2156,7 +2157,7 @@ describe('Settings Module', () => { .replace(//gi, ''); (api.getConfig as jest.Mock).mockResolvedValue({ global: { - enabled_providers: ['aws'], + enabled_providers: providers, notification_email: 'old@test.com', auto_collect: true, collection_schedule: 'daily', @@ -2217,6 +2218,58 @@ describe('Settings Module', () => { expect(globalBody()).toEqual({ enabled_providers: ['aws', 'gcp'] }); }); + describe('refuses a save that leaves no provider enabled (issue #1531)', () => { + const serverProviders = (providers: string[]): void => { + (api.getConfig as jest.Mock).mockResolvedValue({ + global: { enabled_providers: providers, default_term: 3, default_payment: 'all-upfront', default_coverage: 80 }, + services: [], + }); + }; + const uncheckAws = (): void => { + (document.getElementById('provider-aws') as HTMLInputElement).checked = false; + }; + + test('unchecking the only enabled provider is refused with a toast and nothing sent', async () => { + await loadRealPage(['aws']); + mockShowToast.mockClear(); + uncheckAws(); + + await save(); + + expect(mockShowToast).toHaveBeenCalledWith({ message: 'Enable at least one provider before saving.', kind: 'error' }); + expect(api.updateConfig).not.toHaveBeenCalled(); + expect(api.updateServiceConfig).not.toHaveBeenCalled(); + }); + + test('unchecking one of two providers saves the remaining one', async () => { + await loadRealPage(['aws', 'gcp']); + uncheckAws(); + + await save(); + + expect(globalBody()).toEqual({ enabled_providers: ['gcp'] }); + }); + + test('a stored empty list does not block editing an unrelated field', async () => { + await loadRealPage([]); + (document.getElementById('setting-notification-email') as HTMLInputElement).value = 'new@test.com'; + + await save(); + + expect(globalBody()).toEqual({ notification_email: 'new@test.com' }); + }); + + test('another admin enabling gcp makes the merged list non-empty, so the save is allowed', async () => { + await loadRealPage(['aws']); + serverProviders(['gcp']); + uncheckAws(); + + await save(); + + expect(globalBody()).toEqual({ enabled_providers: ['gcp'] }); + }); + }); + test('editing only the collection schedule saves it', async () => { await loadRealPage(); (document.getElementById('setting-collection-schedule') as HTMLSelectElement).value = 'weekly'; diff --git a/frontend/src/settings.ts b/frontend/src/settings.ts index 85b1d0e3..fc7c5d7e 100644 --- a/frontend/src/settings.ts +++ b/frontend/src/settings.ts @@ -3654,6 +3654,12 @@ export async function saveGlobalSettings(e: Event): Promise { if (globalSent) { if ('grace_period_days' in changedGlobal || 'enabled_providers' in changedGlobal) { await rebaseMultiFieldKeys(changedGlobal, isFieldChanged); + // The backend disagrees with itself on an empty list (filters read it as + // "all", the scheduler collects nothing), so never write one (#1531). + if (changedGlobal.enabled_providers?.length === 0) { + showToast({ message: 'Enable at least one provider before saving.', kind: 'error' }); + return; + } } await api.updateConfig(changedGlobal); // Persisted: advance the snapshot now, so a later validation failure or diff --git a/frontend/tests-e2e/settings-save-changed-fields.spec.ts b/frontend/tests-e2e/settings-save-changed-fields.spec.ts index 79c65ff2..94b08d70 100644 --- a/frontend/tests-e2e/settings-save-changed-fields.spec.ts +++ b/frontend/tests-e2e/settings-save-changed-fields.spec.ts @@ -84,3 +84,40 @@ test('Settings Save keeps another admin\'s grace value and refuses an emptied gr expect(puts).toEqual([{ grace_period_days: { aws: 3, azure: 5, gcp: 7 } }]); await expect(page.locator('#setting-grace-aws')).toHaveValue('3'); }); + +// Issue #1531: an empty enabled_providers list is contradictory in the backend +// (filters read it as "all", the scheduler collects nothing), so Save refuses it. +test('Settings Save refuses to leave no provider enabled', async ({ page }) => { + await page.route('**/*', route => new URL(route.request().url()).hostname === '127.0.0.1' + ? route.fallback() : route.abort()); + await seedAuth(page); + await mockApi(page); + const puts: unknown[] = []; + await page.route('**/api/config', route => { + if (route.request().method() === 'PUT') { + puts.push(route.request().postDataJSON()); + return route.fulfill({ json: { status: 'updated' } }); + } + return route.fulfill({ json: { global: { + enabled_providers: ['aws'], notification_email: 'ops@example.com', default_term: 3, default_payment: 'all-upfront', default_coverage: 70, notification_days_before: 3, + }, services: [] } }); + }); + await page.route('**/api/config/service/**', route => { + puts.push(route.request().postDataJSON()); + return route.fulfill({ json: { status: 'updated' } }); + }); + await page.route('**/api/commitment-options', route => route.fulfill({ json: { status: 'ok', aws: {} } })); + await page.route('**/api/ri-exchange/config', route => route.fulfill({ json: { + mode: 'manual', auto_exchange_enabled: false, utilization_threshold: 80, + max_payment_per_exchange_usd: 0, max_payment_daily_usd: 0, lookback_days: 30, + } })); + await page.route('**/api/ladder/**', route => route.fulfill({ json: { configs: [], runs: [], events: [], total_count: 0, total_usd_hr: '0.000000' } })); + await page.goto('/admin/general'); + + await expect(page.locator('#provider-aws')).toBeChecked(); + await page.locator('#provider-aws').uncheck(); + await page.locator('#save-settings-btn').click(); + + await expect(page.locator('.toast, [role="status"]').filter({ hasText: 'Enable at least one provider' }).first()).toBeVisible(); + expect(puts).toHaveLength(0); +});