Repository navigation
fix(config): Settings Save from a stale tab reverts the global default term, payment and coverage #539
Description
Summary
saveGlobalSettings builds the PUT /config body from every form field, loaded when the page opened. A tab left open across another change saves its stale default_term, default_payment and default_coverage even when the user edited only an unrelated field. Since #500 the backend propagates a changed global default to every service config, so the stale values are also copied to every service row.
Location (origin/main d2cdc60)
frontend/src/settings.ts:3500-3514builds a fullapi.Configfrom the DOM, includingdefault_term,default_payment,default_coverage.frontend/src/settings.ts:3527await api.updateConfig(settings)sends all of it.frontend/src/settings.ts:3545-3560then PUTs every service config fromloadedServiceConfigs(also loaded at page open).frontend/src/api/settings.ts:34already acceptsPartial<Config>, and the backend merges omitted keys over the stored config.
Failure scenario
- Tab A and tab B both load Settings (global term 3, coverage 80).
- In tab B change global term to 1 and Save; the backend propagates term 1 to every service config.
- In tab A change only the notification email and Save. Tab A sends term 3, coverage 80 and its stale service rows.
- Global term and every service term are back to 3.
Same class as #530 (lookback dropdown re-sending the cached config, fixed in #537). Related: #522 covers the coverage revert from a single tab's own save (propagation undone by the stale service rows, empty coverage saving 0); this issue is the multi-tab case for term, payment and coverage and is fixed by sending only changed fields.
Found in the #537 review.
Fix direction
Track the loaded value of each field and send only the ones the user changed, for the global body and for each service PUT.
Acceptance
Saving an unrelated field from a stale tab leaves the current global and service term, payment and coverage unchanged (test with two sessions or a stubbed GET between load and save).
Activity
cristim commentedon Oct 6, 2026
Status after PR #570 (partial fix, Refs #539): the global PUT now carries only the fields the user changed and is skipped when none changed (verified against the real index.html and the real PUT /api/config handler). Still open on this issue:
mergeServiceConfigoverlays enabled, term, payment and coverage unconditionally, so a stale tab can still revert service rows. Fixing it needs per-service dirty detection plus filter-field tracking, or a backend change.enabled_providersarray, and the backend replaces the whole map/array (handler_config.go ~96-98), so a stale tab reverts the other providers' entries. A partial map cannot be sent safely until the backend merges these keys.IAmJSD commentedon Oct 16, 2026
Released: remaining work needs frontend changes or an API-contract decision (details below)
IAmJSD commentedon Oct 16, 2026
Released. What's left after #570 can't be fixed safely from the backend alone, and I'm leaving
frontend/alone while you do the UI work.mergeServiceConfigoverlays enabled, term, payment and coverage. The frontend sends all of them from the values it loaded when the page opened, so a stale tab still reverts them. A presence-aware backend merge only helps once the frontend sends just the dirty fields.enabled_providers): the frontend sends every provider's value, stale ones included. A per-key merge inhandler_config.gowould still apply the stale ones.The one backend-only fix is optimistic concurrency. The page would load a version or
updated_atfor the config and the service configs, send it with the PUT, and get a 409 when it's stale. That catches stale tabs for every field. It's an API-contract change, though, and the frontend would need to handle the 409 by reloading. Happy to do the backend half if you want that approach.🤖 Generated with Claude Code
cristim commentedon Oct 26, 2026
Status after checking current main:
PUT /api/configsend only changed keys.enabled,term,paymentandcoverageonly when the request includes them.enabled_providersare sent whole, so a stale tab can still revert them. Optimistic concurrency (a 409 on a stale write) would close this at the root.fix(config): propagate the default payment only to providers that acc…
test(settings): pin the service PUT diff against surviving mutations
fix(settings): refuse an empty edited coverage instead of dropping it
fix(settings): match the renamed coverage toast in the empty-coverage…
fix(settings): save only the service configs and keys the user changed (
cristim commentedon Oct 26, 2026
Another remaining item, found in the #1495 review. The bug predates that PR: emptying the grace-period field saves the default 7 without telling the user (
frontend/src/settings.ts~246). An admin who clears the field probably means 0. It's the same 'empty becomes a value' class as the cache window, which #1495 now refuses. It belongs with the still-open grace-period andenabled_providershandling here.cristim commentedon Oct 26, 2026
Follow-ups from the second review of #1495:
paymenttomonthlyon GCP service rows. It needs a migration number above main's highest at merge time.required: trueon the cache-stale-hours inline validator has no test.monthlyliteral in the GCP repair duplicatesnormalizePaymentValue. Reuse it.fix(settings): repair GCP rows holding an AWS payment and refuse an e…
cristim commentedon Oct 27, 2026
Claiming: working on the final part (claude-cc-platform). The plan: rebase the multi-field keys (
grace_period_daysandenabled_providers) onto a freshGET /api/configbefore the PUT, sending only the fields this tab changed, and refuse an emptied grace field instead of saving 7. A PR that closes this issue will follow.fix(settings): decide changed fields from the values captured at Save