diff --git a/frontend/src/__tests__/settings.test.ts b/frontend/src/__tests__/settings.test.ts index 3b753301d..86e891cd2 100644 --- a/frontend/src/__tests__/settings.test.ts +++ b/frontend/src/__tests__/settings.test.ts @@ -6,7 +6,8 @@ import { saveGlobalSettings, resetSettings, setupSettingsHandlers, - copyToClipboard + copyToClipboard, + isUnsavedChanges, } from '../settings'; // Mock the api module @@ -1435,4 +1436,77 @@ describe('Settings Module', () => { expect(bodyText).not.toMatch(/^aws$/m); }); }); + + // ----------------------------------------------------------------------- + // isUnsavedChanges -- no false prompt on pristine load or before load + // (issue #1441) + // ----------------------------------------------------------------------- + // The navigation guard in navigation.ts calls isUnsavedChanges() whenever + // the user leaves the Admin tab. Before the fix, savedSnapshot started as + // an empty object ({}), so comparing any live DOM value against undefined + // always returned true -- causing the "You have unsaved settings changes. + // Leave without saving?" modal to fire even without any edits. + describe('isUnsavedChanges -- navigation guard (issue #1441)', () => { + const baseConfigResponse = { + global: { + enabled_providers: ['aws'] as string[], + notification_email: '', + auto_collect: true, + default_term: 3, + default_payment: 'all-upfront', + default_coverage: 80, + notification_days_before: 3, + recommendations_cache_stale_hours: 24, + recommendations_lookback_days: 7, + grace_period_days: { aws: 7, azure: 7, gcp: 7 }, + laddering_enabled: false, + }, + services: [], + credentials: {}, + }; + + test('returns false before settings have been loaded (empty snapshot -- regression for #1441)', () => { + // Use a fresh module instance so savedSnapshot starts as the empty object + // {} that it is at module initialisation, before any loadGlobalSettings() + // call has run snapshotAllFields(). This is the exact state that caused + // the bug: the navigation guard fired on every first Admin-tab visit + // because `getFieldValue(id) !== undefined` was always true. + // + // Pre-fix: TRACKED_FIELDS.some(id => getFieldValue(id) !== savedSnapshot[id]) + // = TRACKED_FIELDS.some(id => '' !== undefined) = true -> prompt shown (BUG) + // Post-fix: Object.keys(savedSnapshot).length === 0 -> return false (CORRECT) + let freshIsUnsavedChanges!: () => boolean; + jest.isolateModules(() => { + // eslint-disable-next-line @typescript-eslint/no-require-imports + const s = require('../settings') as { isUnsavedChanges: () => boolean }; + freshIsUnsavedChanges = s.isUnsavedChanges; + }); + expect(freshIsUnsavedChanges()).toBe(false); + }); + + test('returns false immediately after settings load without any user edits', async () => { + // loadGlobalSettings() populates all tracked fields from the API response + // and then calls snapshotAllFields() to establish the clean baseline. + // A user who navigates away right after load (no edits) must see no prompt. + (api.getConfig as jest.Mock).mockResolvedValue(baseConfigResponse); + await loadGlobalSettings(); + + expect(isUnsavedChanges()).toBe(false); + }); + + test('returns true after a tracked field is edited post-load', async () => { + // After a clean load + snapshot, mutating a tracked field must be detected + // so the navigation guard can show the prompt for real unsaved changes. + // Verifies the fix does not suppress legitimate dirty-state detection. + (api.getConfig as jest.Mock).mockResolvedValue(baseConfigResponse); + await loadGlobalSettings(); + + // Simulate the user changing the notification email -- a tracked field. + const emailInput = document.getElementById('setting-notification-email') as HTMLInputElement; + emailInput.value = 'edited@example.com'; + emailInput.dispatchEvent(new Event('input')); + + expect(isUnsavedChanges()).toBe(true); + }); + }); }); diff --git a/frontend/src/settings.ts b/frontend/src/settings.ts index c63f30e0f..6aa204da9 100644 --- a/frontend/src/settings.ts +++ b/frontend/src/settings.ts @@ -364,8 +364,21 @@ function updateDirtyMarkers(): void { reflectDirtyState(anyDirty); } -/** Returns true if any tracked field has been changed since the last save/load. */ +/** + * Returns true if any tracked field has been changed since the last save/load. + * + * Returns false when no snapshot has been taken yet (settings are still + * loading or have never loaded successfully). Before the first + * snapshotAllFields() call, savedSnapshot is an empty object, and comparing + * any live DOM value against undefined would always return true -- causing + * spurious "unsaved changes" prompts for users who navigate away before the + * async settings load completes (issue #1441). + */ export function isUnsavedChanges(): boolean { + // savedSnapshot starts as {} and is populated by snapshotAllFields() only + // after loadGlobalSettings() resolves. If it is still empty the form is + // still loading and no edit could have been made yet. + if (Object.keys(savedSnapshot).length === 0) return false; return TRACKED_FIELDS.some(id => getFieldValue(id) !== savedSnapshot[id]); }