rbrooks/Iris-WLED

Watch

Fork 0

Settings page: each card's Save sends the whole form, and a refetch can wipe unsaved edits #181

New issue

Closed

opened 2026-09-26 13:31:14 +00:00 by claude-bot · 1 comment

claude-bot commented

2026-09-26 13:31:14 +00:00

Collaborator

Found while diagnosing #180's CI failure (it didn't cause it). Two related problems on SettingsPage.tsx.

1. Every card's Save sends all 46 keys

saveSettings() sends the entire form, not just the card's own fields. So saving the Watchdog card also writes the WLED host, the location, every schedule time, the AI provider, and everything else, as whatever this browser tab last loaded.

Two people, or two tabs, each saving a different card overwrite each other's changes with stale values.
It pins every runtime key into the database on the first save of any card. After that, .env seeds can't take effect for any key (see Rhoving/iac-repo#527 §6.1), not just the one the person meant to change.

Fix: each card sends only its own keys. The backend set_all_settings already persists only the keys it's given.

2. A refetch overwrites unsaved edits

The page copies settings into local form state in a useEffect keyed on settings. When the settings query returns new data (a save from another tab or device, or an invalidation after some other mutation), the effect reruns and silently discards whatever the user has typed but not saved. React Query's structural sharing prevents this when nothing changed, so it only bites when something really did change.

Fix: track dirty fields per card, and don't overwrite a field the user has edited since the last load. Optionally show "Settings changed elsewhere — reload?".

Tests

A vitest where a settings refetch with changed data arrives while a card has unsaved edits, and the edits survive. And a test that saving the Watchdog card sends only watchdog_* keys.

Found while diagnosing #180's CI failure (it didn't cause it). Two related problems on `SettingsPage.tsx`. ## 1. Every card's Save sends all 46 keys `saveSettings()` sends the entire form, not just the card's own fields. So saving the Watchdog card also writes the WLED host, the location, every schedule time, the AI provider, and everything else, as whatever this browser tab last loaded. - **Two people, or two tabs,** each saving a different card overwrite each other's changes with stale values. - **It pins every runtime key into the database on the first save of any card.** After that, `.env` seeds can't take effect for any key (see Rhoving/iac-repo#527 §6.1), not just the one the person meant to change. **Fix:** each card sends only its own keys. The backend `set_all_settings` already persists only the keys it's given. ## 2. A refetch overwrites unsaved edits The page copies `settings` into local form state in a `useEffect` keyed on `settings`. When the settings query returns new data (a save from another tab or device, or an invalidation after some other mutation), the effect reruns and silently discards whatever the user has typed but not saved. React Query's structural sharing prevents this when nothing changed, so it only bites when something really did change. **Fix:** track dirty fields per card, and don't overwrite a field the user has edited since the last load. Optionally show "Settings changed elsewhere — reload?". ## Tests A vitest where a settings refetch with changed data arrives while a card has unsaved edits, and the edits survive. And a test that saving the Watchdog card sends only `watchdog_*` keys.

claude-bot added this to the v1.2.0 milestone

2026-09-26 13:31:24 +00:00

claude-bot added the

labels

2026-09-26 13:31:25 +00:00

rbrooks referenced this issue from a commit

2026-09-26 15:14:23 +00:00

Keep Settings edits until they are saved (#181)

~~claude-bot referenced this issue 2026-09-26 15:14:48 +00:00~~

Show what the lights used and cost this season (#153), and keep Settings edits until saved (#181) #180

claude-bot referenced this issue from a pull request that will close it,

2026-09-26 15:14:49 +00:00

Show what the lights used and cost this season (#153), and keep Settings edits until saved (#181) #180

claude-bot closed this issue

2026-09-26 15:41:11 +00:00

claude-bot referenced this issue from a commit

2026-09-26 15:41:11 +00:00

Show what the lights used and cost this season (#153), and keep Settings edits until saved (#181) (#180)

claude-bot referenced this issue

2026-09-26 15:41:23 +00:00

Energy and cost report: what the lights actually used this season #153

claude-bot commented

2026-09-26 15:41:48 +00:00

Author

Collaborator

Fixed in #180 (commit 6574b4c, squash-merged, CI green).

Root cause, confirmed by instrumentation: the Settings page renders its cards before GET /settings returns, and a useEffect([settings]) copied every server value over the form when the response landed. An edit made during that window was silently lost, and Save sent the old values. It wasn't a remount or the service worker.

Fix:

New useDrafts / useDraft: a field the person has edited is never overwritten by a load or refetch.
Save sends only the edited settings, so one card no longer rewrites the others with stale values or pins every key into the database. That covers both halves of this issue.
The same guard for the segment list and webhook drafts.
Also fixed: a render loop where data: webhooks = [] created a new array on every render, which re-rendered forever for viewers.

Tests: settingsDrafts.test.tsx (6 tests), shown to fail on the old page, and the watchdog E2E asserts the exact PUT body. 16 post-fix E2E runs under CPU limits had zero reverts.

Fixed in #180 (commit `6574b4c`, squash-merged, CI green). **Root cause, confirmed by instrumentation:** the Settings page renders its cards before `GET /settings` returns, and a `useEffect([settings])` copied every server value over the form when the response landed. An edit made during that window was silently lost, and Save sent the old values. It wasn't a remount or the service worker. **Fix:** - **New `useDrafts` / `useDraft`:** a field the person has edited is never overwritten by a load or refetch. - **Save sends only the edited settings,** so one card no longer rewrites the others with stale values or pins every key into the database. That covers both halves of this issue. - **The same guard for the segment list and webhook drafts.** - **Also fixed:** a render loop where `data: webhooks = []` created a new array on every render, which re-rendered **forever for viewers**. **Tests:** `settingsDrafts.test.tsx` (6 tests), shown to fail on the old page, and the watchdog E2E asserts the exact PUT body. 16 post-fix E2E runs under CPU limits had zero reverts.

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

rbrooks/Iris-WLED#181

No description provided.

Rows
Columns