Repository navigation
fix(admin): only prompt for unsaved changes when settings actually edited (closes #1441) - #1445
Conversation
…ited (closes #1441) isUnsavedChanges() compared live DOM values against savedSnapshot, which starts as an empty object ({}) before loadGlobalSettings() resolves. Since any non-undefined DOM value differs from undefined, the comparison always returned true, so the "You have unsaved settings changes. Leave without saving?" prompt fired on every Admin-tab navigation -- even for read-only users who had made no edits and even before the async settings load completed. Fix: guard with Object.keys(savedSnapshot).length === 0 at the top of isUnsavedChanges(). When the snapshot is empty the form is still loading and no edit is possible; return false immediately. snapshotAllFields() (called inside the loadGlobalSettings() try block after all fields are populated) sets the snapshot, enabling normal dirty-comparison for subsequent checks. Add three regression tests: the empty-snapshot path (via jest.isolateModules to get a fresh module instance), pristine-load navigation (no prompt), and edit-then-navigate (prompt shown correctly).
|
@coderabbitai review |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe settings navigation guard now returns no unsaved-change state before settings loading completes. Tests cover fresh module state, a loaded unchanged form, and a modified tracked field, with deterministic confirmation and toast mocks. ChangesSettings navigation guard
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
✅ Action performedReview finished.
|
|
@coderabbitai full review |
✅ Action performedFull review finished. |
Root cause
isUnsavedChanges()insettings.tscompared live DOM values againstsavedSnapshot, which starts as an empty object ({}) at module initialisation.loadGlobalSettings()is async -- it callssnapshotAllFields()only after the API response arrives and all fields are populated. During the async window between the navigation-guard being armed and the snapshot being populated, every comparison wasgetFieldValue(id) !== undefinedwhich is alwaystrue. This caused the "You have unsaved settings changes. Leave without saving?" modal to fire on every Admin-tab navigation, for all roles including read-only viewers who had made no edits.Fix
Guard
isUnsavedChanges()withObject.keys(savedSnapshot).length === 0 → return false. When the snapshot is empty the form is still loading and no edit is possible. AftersnapshotAllFields()runs (insideloadGlobalSettings()after all fields are populated), the snapshot is non-empty and dirty comparison proceeds normally.Tests added (
frontend/src/__tests__/settings.test.ts)jest.isolateModulesfor a fresh module instance): fails with pre-fix code (undefinedcomparison returns true), passes with fix.isUnsavedChanges()returnsfalseafterloadGlobalSettings()with no user edits.isUnsavedChanges()returnstrueafter a tracked field is mutated, confirming the fix does not suppress legitimate dirty detection.Test plan
cd frontend && npx jest --no-coverage --testPathPattern="settings.test.ts"-- all 75 tests pass.npm run buildsucceeds.Closes #1441
Summary by CodeRabbit