From f347d87b64ac47e8bb8b8fc312ccb4b5d8f70367 Mon Sep 17 00:00:00 2001 From: CallMeTechie <34693633+CallMeTechie@users.noreply.github.com> Date: Thu, 25 Jun 2026 10:23:19 +0200 Subject: [PATCH] fix(settings): resync autosave dirty-snapshot after async load; align valuesById keys to element ids MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The autosave controller captured its baseline snapshot synchronously at bind time, before any async load*() populated field values from the server. A user changing a field back to its template default was seen as "not dirty" and the PUT was silently dropped. Part A – controller: add a binds registry, a resync() closure per cluster, and expose window.SettingsAutosave.resync(cluster) so callers can refresh the snapshot after async population. bind() also returns { resync } for direct use. Part B – callers: add SettingsAutosave.resync('X') at the end of every async loader that populates field values after the synchronous bind. Affected clusters: smtp, security, data, monitoring, alerts, autobackup, metrics, dns, auto-update, split-tunnel, pihole, portal, route-block. machine-binding is already correct (bind is inside the async IIFE after the await + value set). gateway-failover is server-rendered with no async load. Part C – key alignment: dns valuesById key was 'dns' (vs element id 'settings-dns-input'); gateway-failover was 'gw' (vs 'gw-down-threshold'). Both produced spurious [autosave] … missing bound fields console warnings. Keys changed to match element ids; save() payloads are unchanged. Tests: three new smoke assertions cover the resync registry, per-cluster resync call presence, and the corrected key names. Full suite: 23 autosave + 48 api tests green. --- public/js/settings.js | 21 ++++++++++++++++++--- public/js/settingsAutosave.js | 6 ++++++ tests/settings_autosave_smoke.test.js | 25 +++++++++++++++++++++++++ 3 files changed, 49 insertions(+), 3 deletions(-) diff --git a/public/js/settings.js b/public/js/settings.js index 1aea212a..681f1e7f 100644 --- a/public/js/settings.js +++ b/public/js/settings.js @@ -308,6 +308,7 @@ hint.textContent = (window.GC.t || {})['settings.smtp.password_set'] || 'Password is set'; hint.style.display = ''; } + if (window.SettingsAutosave && SettingsAutosave.resync) SettingsAutosave.resync('smtp'); } }).catch(function(err) { console.error('Failed to load SMTP settings:', err); @@ -457,6 +458,7 @@ if (pwNum) { if (pw.require_number) pwNum.classList.add('on'); else pwNum.classList.remove('on'); } var pwSpecial = document.getElementById('security-password-special'); if (pwSpecial) { if (pw.require_special) pwSpecial.classList.add('on'); else pwSpecial.classList.remove('on'); } + if (window.SettingsAutosave && SettingsAutosave.resync) SettingsAutosave.resync('security'); } catch (err) { console.error('Failed to load security settings:', err); } @@ -569,6 +571,7 @@ if (el2) el2.value = d.retention_activity_days; var el3 = document.getElementById('data-peer-timeout'); if (el3) el3.value = d.peer_online_timeout; + if (window.SettingsAutosave && SettingsAutosave.resync) SettingsAutosave.resync('data'); } catch (err) { console.error('Failed to load data settings:', err); } @@ -621,6 +624,7 @@ if (monEmailToggle) { if (d.emailAlerts) monEmailToggle.classList.add('on'); else monEmailToggle.classList.remove('on'); } var emailEl = document.getElementById('monitoring-alert-email'); if (emailEl) emailEl.value = d.alertEmail || ''; + if (window.SettingsAutosave && SettingsAutosave.resync) SettingsAutosave.resync('monitoring'); } catch (err) { console.error('Failed to load monitoring settings:', err); } @@ -675,6 +679,7 @@ var groupEvents = cb.dataset.events.split(','); cb.checked = groupEvents.some(function(e) { return configuredEvents.includes(e); }); }); + if (window.SettingsAutosave && SettingsAutosave.resync) SettingsAutosave.resync('alerts'); } catch (err) { console.error('Failed to load alert settings:', err); } @@ -839,6 +844,7 @@ ? new Date(d.lastRun).toLocaleString() : (GC.t['autobackup.last_run_never'] || 'Never'); } + if (window.SettingsAutosave && SettingsAutosave.resync) SettingsAutosave.resync('autobackup'); } catch (err) { console.error('Failed to load auto-backup settings:', err); } @@ -983,6 +989,7 @@ if (data.data.enabled) metricsEnabledToggle.classList.add('on'); else metricsEnabledToggle.classList.remove('on'); } + if (window.SettingsAutosave && SettingsAutosave.resync) SettingsAutosave.resync('metrics'); } catch (err) { console.error('Failed to load metrics settings:', err); } @@ -1080,14 +1087,17 @@ if (dnsInput) { api.get('/api/v1/settings/dns').then(function(data) { - if (data.ok) dnsInput.value = data.data.dns || ''; + if (data.ok) { + dnsInput.value = data.data.dns || ''; + if (window.SettingsAutosave && SettingsAutosave.resync) SettingsAutosave.resync('dns'); + } }).catch(function() {}); SettingsAutosave.bind({ cluster: 'dns', fields: [dnsInput], statusEl: document.getElementById('dns-status'), - valuesById: function () { return { dns: dnsInput.value.trim() }; }, + valuesById: function () { return { 'settings-dns-input': dnsInput.value.trim() }; }, save: function () { return api.put('/api/v1/settings/dns', { dns: dnsInput.value.trim() }); }, }); } @@ -1100,6 +1110,7 @@ window.api.get('/api/system/auto-update').then(function (d) { var el = card.querySelector('input[name="au-mode"][value="' + ((d && d.mode) || 'auto') + '"]'); if (el) el.checked = true; + if (window.SettingsAutosave && SettingsAutosave.resync) SettingsAutosave.resync('auto-update'); }).catch(function () {}); var auRadios = Array.prototype.slice.call(document.querySelectorAll('input[name="au-mode"]')); if (auRadios.length) { @@ -1346,6 +1357,7 @@ linkLocal.checked = nets.some(function (n) { return n.cidr === LINK_LOCAL.cidr; }); customNets = nets.filter(function (n) { return pCidrs.indexOf(n.cidr) < 0 && n.cidr !== LINK_LOCAL.cidr; }); renderCustom(); + if (window.SettingsAutosave && SettingsAutosave.resync) SettingsAutosave.resync('split-tunnel'); } catch {} } @@ -1415,7 +1427,7 @@ cluster: 'gateway-failover', fields: [sliderEl], statusEl: document.getElementById('gw-failover-status'), - valuesById: function () { return { gw: sliderEl.value }; }, + valuesById: function () { return { 'gw-down-threshold': sliderEl.value }; }, save: function () { return api.put('/api/v1/settings/gateway-failover', { gateway_down_threshold_s: parseInt(sliderEl.value, 10) }); }, }); } @@ -1445,6 +1457,7 @@ if (intervalEl) intervalEl.value = cfg.sync_interval_sec || 30; phInstances = (cfg.instances || []).slice(); renderInstances(); + if (window.SettingsAutosave && SettingsAutosave.resync) SettingsAutosave.resync('pihole'); } catch (err) { console.error('Failed to load Pi-hole settings:', err); } @@ -1771,6 +1784,7 @@ setToggle(widgetDevice, d.widgets && d.widgets.device); setToggle(widgetTraffic, d.widgets && d.widgets.traffic); setToggle(widgetServices, d.widgets && d.widgets.services); + if (window.SettingsAutosave && SettingsAutosave.resync) SettingsAutosave.resync('portal'); }).catch(function (err) { console.error('Failed to load portal settings:', err); }); @@ -1891,6 +1905,7 @@ if (bodyEl) bodyEl.value = r.data.body || ''; if (redirectEl) redirectEl.value = r.data.redirect_url || ''; syncSettingsBlockVisibility(); + if (window.SettingsAutosave && SettingsAutosave.resync) SettingsAutosave.resync('route-block'); } }).catch(function (err) { console.error('Failed to load route block default:', err); diff --git a/public/js/settingsAutosave.js b/public/js/settingsAutosave.js index 6a41fb4e..61bde58a 100644 --- a/public/js/settingsAutosave.js +++ b/public/js/settingsAutosave.js @@ -3,6 +3,7 @@ var Core = window.SettingsAutosaveCore; var enqueue = Core.createQueue(); // shared per-cluster serialization var t = (window.GC && window.GC.t) || {}; + var binds = {}; // registry of bound clusters → { resync } window.SettingsAutosave = { enqueue: enqueue }; function flash(statusEl) { @@ -44,6 +45,8 @@ var statusEl = opts.statusEl || null; var valuesById = opts.valuesById || function () { return {}; }; var snapshot = JSON.stringify(valuesById()); // last successfully persisted state + function resync() { snapshot = JSON.stringify(valuesById()); } + if (cluster) binds[cluster] = { resync: resync }; // Dev guard: warn if any bound field id is absent from valuesById (would never be dirty). var _boundIds = fields.map(function(f) { return f && f.id; }).filter(Boolean); @@ -93,7 +96,10 @@ }); } }); + + return { resync: resync }; } window.SettingsAutosave.bind = bind; + window.SettingsAutosave.resync = function (cluster) { if (binds[cluster] && binds[cluster].resync) binds[cluster].resync(); }; })(); diff --git a/tests/settings_autosave_smoke.test.js b/tests/settings_autosave_smoke.test.js index bb31e8fa..5f001a8e 100644 --- a/tests/settings_autosave_smoke.test.js +++ b/tests/settings_autosave_smoke.test.js @@ -104,3 +104,28 @@ test('two concurrent pihole PUTs leave a deterministic (non-corrupted) DB state' license._overrideForTest({ pihole_integration: false }); } }); + +test('resync mechanism: controller exposes resync and keeps a binds registry', async () => { + const ctrl = await supertest(app).get('/js/settingsAutosave.js').expect(200); + assert.match(ctrl.text, /SettingsAutosave\.resync/); + assert.match(ctrl.text, /binds\[/); + assert.match(ctrl.text, /function resync\(\)/); +}); + +test('settings.js calls SettingsAutosave.resync for all async-populated clusters', async () => { + const js = await supertest(app).get('/js/settings.js').expect(200); + const clusters = ['smtp', 'security', 'data', 'monitoring', 'alerts', 'autobackup', 'metrics', 'dns', 'auto-update', 'split-tunnel', 'pihole', 'portal', 'route-block']; + clusters.forEach(function (c) { + assert.match(js.text, new RegExp("SettingsAutosave\\.resync\\('" + c.replace('-', '\\-') + "'\\)"), 'missing resync for cluster: ' + c); + }); +}); + +test('valuesById keys align with element ids: dns uses settings-dns-input, gateway-failover uses gw-down-threshold', async () => { + const js = await supertest(app).get('/js/settings.js').expect(200); + // dns valuesById must use the element id, not the semantic payload key 'dns' + assert.match(js.text, /'settings-dns-input': dnsInput/); + // gateway-failover valuesById must use the element id + assert.match(js.text, /'gw-down-threshold': sliderEl/); + // old semantic key 'gw' must be gone from valuesById (return { gw: sliderEl... }) + assert.doesNotMatch(js.text, /return \{ gw: sliderEl/); +});