From a59a74c9767f24b9972e4498a7bbd665bcb04e35 Mon Sep 17 00:00:00 2001 From: thePR0M3TH3AN <53631862+PR0M3TH3AN@users.noreply.github.com> Date: Wed, 1 Jul 2026 16:04:33 -0400 Subject: [PATCH] [sync] Unify wallet + storage sync UI: one confirm, consistent notifications (step 2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ProfileWalletController and ProfileStorageController each had their own copy of the sync toggle/restore/overwrite-confirm logic, producing different notifications and duplicate confirm dialogs. Extract js/ui/profileModal/syncSection.js — one overwrite-confirm dialog, one status-class mapping, and shared runSyncToggle / runSyncRestore handlers. Both controllers now delegate, so every syncable item gets identical toasts + one confirm (conflict-only, via the shared syncConflict). Intentional divergence preserved: the NWC spending-secret warning stays as the wallet's preEnableConfirm (a wallet-connect URI can spend), since publishing it even encrypted warrants a one-time heads-up. Tests: tests/sync-section.test.mjs (8) — enable/conflict/nothing-to-sync/rollback/ preEnableConfirm/disable/restore, plus wallet-vs-storage produce the same message shapes. wallet-sync + storage-sync + profile-modal suites green (34). Co-Authored-By: Claude Opus 4.8 --- .../profileModal/ProfileStorageController.js | 93 +++++--------- js/ui/profileModal/ProfileWalletController.js | 114 ++++++----------- js/ui/profileModal/syncSection.js | 118 ++++++++++++++++++ tests/sync-section.test.mjs | 111 ++++++++++++++++ 4 files changed, 298 insertions(+), 138 deletions(-) create mode 100644 js/ui/profileModal/syncSection.js create mode 100644 tests/sync-section.test.mjs diff --git a/js/ui/profileModal/ProfileStorageController.js b/js/ui/profileModal/ProfileStorageController.js index 7f95fabf..b2798d1c 100644 --- a/js/ui/profileModal/ProfileStorageController.js +++ b/js/ui/profileModal/ProfileStorageController.js @@ -10,7 +10,14 @@ import { getActiveSigner } from "../../nostr/client.js"; import { DEFAULT_NIP07_ENCRYPTION_METHODS } from "../../nostr/nip07Permissions.js"; import { storageSyncService } from "../../services/storageSyncService.js"; import { StorageCorsHelp } from "./storageCorsHelp.js"; -import { showConfirm } from "../confirmDialog.js"; +import { + syncStatusClass, + runSyncToggle, + runSyncRestore, + confirmSyncOverwrite, +} from "./syncSection.js"; + +const STORAGE_SYNC_LABEL = "storage settings"; import { fillStorageForm, clearCredentialFields, @@ -241,14 +248,6 @@ export class ProfileStorageController { this.updateStorageFormVisibility(); } - confirmSyncOverwrite() { - return showConfirm( - "A newer copy of your storage settings is on your account (changed on " + - "another device). Overwrite it with this one?", - { confirmLabel: "Overwrite", danger: true }, - ); - } - renderSyncSection(pubkey) { if (!(this.storageSyncSection instanceof HTMLElement)) { return; @@ -271,13 +270,7 @@ export class ProfileStorageController { return; } this.storageSyncStatus.textContent = message || ""; - const toneClass = - tone === "success" - ? "text-status-success" - : tone === "error" - ? "text-status-danger" - : "text-muted"; - this.storageSyncStatus.className = `text-xs ${toneClass}`; + this.storageSyncStatus.className = syncStatusClass(tone); } async handleToggleSync() { @@ -292,41 +285,20 @@ export class ProfileStorageController { ? this.storageSyncToggle.checked : false; try { - if (enabled) { - this.setSyncStatus("Encrypting and publishing…"); - const result = await storageSyncService.enable(pubkey, { - confirmOverwrite: () => this.confirmSyncOverwrite(), - }); - if (result?.ok) { - this.setSyncStatus( - `Synced to ${result.accepted}/${result.total} relays.`, - "success" - ); - this.mainController.showSuccess("Storage settings synced (encrypted)."); - } else if (result?.conflict) { - // User declined to overwrite a newer copy from another device. - this.setSyncStatus( - "Kept the newer copy on your account. Use Restore to pull it, or save again to overwrite.", - ); - } else { - // Roll the toggle back so it reflects reality. + await runSyncToggle({ + service: storageSyncService, + pubkey, + enabled, + itemLabel: STORAGE_SYNC_LABEL, + emptyHint: "Nothing to sync yet — save a connection first.", + setStatus: (msg, tone) => this.setSyncStatus(msg, tone), + showSuccess: (msg) => this.mainController.showSuccess(msg), + setToggle: (value) => { if (this.storageSyncToggle instanceof HTMLInputElement) { - this.storageSyncToggle.checked = false; + this.storageSyncToggle.checked = value; } - await storageSyncService.disable(pubkey).catch(() => {}); - this.setSyncStatus( - result?.error === "nothing-to-sync" - ? "Nothing to sync yet — save a connection first." - : "Could not publish the encrypted copy. Try again.", - "error" - ); - } - } else { - this.setSyncStatus("Removing the synced copy…"); - await storageSyncService.disable(pubkey); - this.setSyncStatus("Sync turned off; the synced copy was cleared."); - this.mainController.showSuccess("Storage sync turned off."); - } + }, + }); } catch (error) { devLogger.error("[ProfileModal] Storage sync toggle failed:", error); this.setSyncStatus("Sync failed. Please try again.", "error"); @@ -341,20 +313,15 @@ export class ProfileStorageController { return; } try { - this.setSyncStatus("Fetching and decrypting…"); - const result = await storageSyncService.pull(pubkey); - if (result?.found && result.imported) { - this.setSyncStatus("Restored from your Nostr account.", "success"); - this.mainController.showSuccess("Storage settings restored."); + await runSyncRestore({ + service: storageSyncService, + pubkey, + itemLabel: STORAGE_SYNC_LABEL, + setStatus: (msg, tone) => this.setSyncStatus(msg, tone), + showSuccess: (msg) => this.mainController.showSuccess(msg), // Re-render the pane so the restored connection shows. - await this.populateStoragePane(); - } else if (result?.found && !result.imported) { - this.setSyncStatus("Found a copy but could not import it.", "error"); - } else if (result?.cleared) { - this.setSyncStatus("No synced settings found (it was cleared)."); - } else { - this.setSyncStatus("No synced settings found on your account."); - } + onImported: () => this.populateStoragePane(), + }); } catch (error) { devLogger.error("[ProfileModal] Storage sync restore failed:", error); this.setSyncStatus("Restore failed. Please try again.", "error"); @@ -861,7 +828,7 @@ export class ProfileStorageController { if (storageSyncService.isEnabled(pubkey)) { try { const syncResult = await storageSyncService.push(pubkey, { - confirmOverwrite: () => this.confirmSyncOverwrite(), + confirmOverwrite: () => confirmSyncOverwrite(STORAGE_SYNC_LABEL), }); if (syncResult?.ok) { this.setSyncStatus("Synced copy updated.", "success"); diff --git a/js/ui/profileModal/ProfileWalletController.js b/js/ui/profileModal/ProfileWalletController.js index 17a90f47..7ddd35d3 100644 --- a/js/ui/profileModal/ProfileWalletController.js +++ b/js/ui/profileModal/ProfileWalletController.js @@ -3,6 +3,14 @@ import { showConfirm } from "../confirmDialog.js"; import { NWC_URI_SCHEME } from "../profileModalContract.js"; import { createWalletSyncService } from "../../services/walletSyncService.js"; import { runWalletZappabilityCheck } from "./walletZappabilityCheck.js"; +import { + syncStatusClass, + runSyncToggle, + runSyncRestore, + confirmSyncOverwrite, +} from "./syncSection.js"; + +const WALLET_SYNC_LABEL = "wallet connection"; const noop = () => {}; const SECRET_PLACEHOLDER = "*****"; @@ -104,14 +112,6 @@ export class ProfileWalletController { return this._walletSyncService; } - confirmSyncOverwrite() { - return showConfirm( - "A newer copy of your wallet connection is on your account (changed on " + - "another device). Overwrite it with this one?", - { confirmLabel: "Overwrite", danger: true }, - ); - } - renderSyncSection(pubkey) { if (!(this.walletSyncSection instanceof HTMLElement)) { return; @@ -133,13 +133,7 @@ export class ProfileWalletController { return; } this.walletSyncStatus.textContent = message || ""; - const toneClass = - tone === "success" - ? "text-status-success" - : tone === "error" - ? "text-status-danger" - : "text-muted"; - this.walletSyncStatus.className = `text-xs ${toneClass}`; + this.walletSyncStatus.className = syncStatusClass(tone); } async handleToggleSync() { @@ -155,54 +149,29 @@ export class ProfileWalletController { ? this.walletSyncToggle.checked : false; try { - if (enabled) { - // Spending capability — require explicit confirmation before publishing. - const confirmed = await showConfirm( - "This publishes an ENCRYPTED copy of your wallet connection to public relays.\n\n" + - "A wallet connect URI can SPEND from your wallet. Anyone who controls your " + - "Nostr key could decrypt this and spend from this wallet.\n\nContinue?", - { confirmLabel: "Publish", danger: true }, - ); - if (!confirmed) { + await runSyncToggle({ + service: sync, + pubkey, + enabled, + itemLabel: WALLET_SYNC_LABEL, + emptyHint: "Connect a wallet first, then enable sync.", + setStatus: (msg, tone) => this.setSyncStatus(msg, tone), + showSuccess: (msg) => this.mainController.showSuccess(msg), + setToggle: (value) => { if (this.walletSyncToggle instanceof HTMLInputElement) { - this.walletSyncToggle.checked = false; + this.walletSyncToggle.checked = value; } - return; - } - this.setSyncStatus("Encrypting and publishing…"); - const result = await sync.enable(pubkey, { - confirmOverwrite: () => this.confirmSyncOverwrite(), - }); - if (result?.ok) { - this.setSyncStatus( - `Synced to ${result.accepted}/${result.total} relays.`, - "success" - ); - this.mainController.showSuccess("Wallet connection synced (encrypted)."); - } else if (result?.conflict) { - // User declined to overwrite a newer copy from another device. Keep - // sync on (flag set) and the newer remote intact. - this.setSyncStatus( - "Kept the newer copy on your account. Use Restore to pull it, or save again to overwrite.", - ); - } else { - if (this.walletSyncToggle instanceof HTMLInputElement) { - this.walletSyncToggle.checked = false; - } - await sync.disable(pubkey).catch(() => {}); - this.setSyncStatus( - result?.error === "nothing-to-sync" - ? "Connect a wallet first, then enable sync." - : "Could not publish the encrypted copy. Try again.", - "error" - ); - } - } else { - this.setSyncStatus("Removing the synced copy…"); - await sync.disable(pubkey); - this.setSyncStatus("Sync turned off; the synced copy was cleared."); - this.mainController.showSuccess("Wallet sync turned off."); - } + }, + // Spending capability — a wallet-connect URI can SPEND. Keep the explicit + // one-time warning before publishing (even encrypted) to public relays. + preEnableConfirm: () => + showConfirm( + "This publishes an ENCRYPTED copy of your wallet connection to public relays.\n\n" + + "A wallet connect URI can SPEND from your wallet. Anyone who controls your " + + "Nostr key could decrypt this and spend from this wallet.\n\nContinue?", + { confirmLabel: "Publish", danger: true }, + ), + }); } catch (error) { devLogger.error("[ProfileModal] Wallet sync toggle failed:", error); this.setSyncStatus("Sync failed. Please try again.", "error"); @@ -218,19 +187,14 @@ export class ProfileWalletController { } const sync = this.getWalletSyncService(); try { - this.setSyncStatus("Fetching and decrypting…"); - const result = await sync.pull(pubkey); - if (result?.found && result.imported) { - this.setSyncStatus("Restored from your Nostr account.", "success"); - this.mainController.showSuccess("Wallet connection restored."); - this.refreshWalletPaneState(); - } else if (result?.found && !result.imported) { - this.setSyncStatus("Found a copy but could not import it.", "error"); - } else if (result?.cleared) { - this.setSyncStatus("No synced wallet found (it was cleared)."); - } else { - this.setSyncStatus("No synced wallet found on your account."); - } + await runSyncRestore({ + service: sync, + pubkey, + itemLabel: WALLET_SYNC_LABEL, + setStatus: (msg, tone) => this.setSyncStatus(msg, tone), + showSuccess: (msg) => this.mainController.showSuccess(msg), + onImported: () => this.refreshWalletPaneState(), + }); } catch (error) { devLogger.error("[ProfileModal] Wallet sync restore failed:", error); this.setSyncStatus("Restore failed. Please try again.", "error"); @@ -641,7 +605,7 @@ export class ProfileWalletController { if (sync.isEnabled(normalizedActive)) { try { const syncResult = await sync.push(normalizedActive, { - confirmOverwrite: () => this.confirmSyncOverwrite(), + confirmOverwrite: () => confirmSyncOverwrite(WALLET_SYNC_LABEL), }); if (syncResult?.conflict) { this.setSyncStatus( diff --git a/js/ui/profileModal/syncSection.js b/js/ui/profileModal/syncSection.js new file mode 100644 index 00000000..157870bc --- /dev/null +++ b/js/ui/profileModal/syncSection.js @@ -0,0 +1,118 @@ +// Shared UI for a syncable profile item (NWC wallet, storage credentials, …). +// Previously ProfileWalletController and ProfileStorageController each had their +// own copy of the sync-toggle / restore / overwrite-confirm logic, which produced +// inconsistent notifications and duplicate confirm dialogs. This module is the one +// implementation both use, so every item gets identical toasts, one overwrite +// confirm, and the same conflict-only behavior. Item-specific bits (the DOM status +// element, the "empty" hint, an optional pre-enable warning, and the re-render +// after restore) are passed in. + +import { showConfirm } from "../confirmDialog.js"; + +function capitalize(label) { + const value = typeof label === "string" ? label.trim() : ""; + return value ? value[0].toUpperCase() + value.slice(1) : value; +} + +// The ONE overwrite-confirm dialog, shown only on a real conflict (remote is +// newer — changed on another device). itemLabel = "wallet connection" | "storage +// settings". +export function confirmSyncOverwrite(itemLabel) { + return showConfirm( + `A newer copy of your ${itemLabel} is on your account (changed on another ` + + `device). Overwrite it with this one?`, + { confirmLabel: "Overwrite", danger: true }, + ); +} + +// tone → the status line's className (consistent across items). +export function syncStatusClass(tone) { + const toneClass = + tone === "success" + ? "text-status-success" + : tone === "error" + ? "text-status-danger" + : "text-muted"; + return `text-xs ${toneClass}`; +} + +// Shared enable/disable handler. `setToggle` reflects reality on rollback; +// `preEnableConfirm` is an optional async gate (e.g. the wallet spending-secret +// warning); `emptyHint` is the "nothing-to-sync" message. +export async function runSyncToggle({ + service, + pubkey, + enabled, + itemLabel, + setStatus, + showSuccess, + setToggle, + preEnableConfirm, + emptyHint = "Nothing to sync yet — save something first.", +}) { + if (enabled) { + if (typeof preEnableConfirm === "function") { + const confirmed = await preEnableConfirm(); + if (!confirmed) { + setToggle?.(false); + return { ok: false, cancelled: true }; + } + } + setStatus("Encrypting and publishing…"); + const result = await service.enable(pubkey, { + confirmOverwrite: () => confirmSyncOverwrite(itemLabel), + }); + if (result?.ok) { + setStatus(`Synced to ${result.accepted}/${result.total} relays.`, "success"); + showSuccess?.(`${capitalize(itemLabel)} synced (encrypted).`); + } else if (result?.conflict) { + // Declined to overwrite a newer remote copy — keep sync on, remote intact. + setStatus( + "Kept the newer copy on your account. Use Restore to pull it, or save again to overwrite.", + ); + } else { + setToggle?.(false); + await service.disable(pubkey).catch(() => {}); + setStatus( + result?.error === "nothing-to-sync" + ? emptyHint + : "Could not publish the encrypted copy. Try again.", + "error", + ); + } + return result; + } + + setStatus("Removing the synced copy…"); + await service.disable(pubkey); + setStatus("Sync turned off; the synced copy was cleared."); + showSuccess?.(`${capitalize(itemLabel)} sync turned off.`); + return { ok: true, disabled: true }; +} + +// Shared restore (pull) handler. `onImported` re-renders the item's pane. +export async function runSyncRestore({ + service, + pubkey, + itemLabel, + setStatus, + showSuccess, + onImported, +}) { + setStatus("Fetching and decrypting…"); + const result = await service.pull(pubkey); + if (result?.found && result.imported) { + setStatus("Restored from your Nostr account.", "success"); + showSuccess?.(`${capitalize(itemLabel)} restored.`); + if (typeof onImported === "function") { + await onImported(); + } + } else if (result?.found && !result.imported) { + setStatus("Found a copy but could not import it.", "error"); + } else if (result?.cleared) { + setStatus("No synced settings found (it was cleared)."); + } else { + setStatus("No synced settings found on your account."); + } + return result; +} diff --git a/tests/sync-section.test.mjs b/tests/sync-section.test.mjs new file mode 100644 index 00000000..461f0e12 --- /dev/null +++ b/tests/sync-section.test.mjs @@ -0,0 +1,111 @@ +// The wallet + storage profile sections now share one sync UI implementation +// (syncSection.js) so they give identical notifications and one overwrite confirm, +// with conflict-only prompting. This covers the shared enable/disable/restore +// handlers (the run* helpers) that both controllers delegate to. +// +// test_integrity_note: +// change_type: ["new_tests"] +// scenarios: +// - id: SCN-sync-section-shared-handlers +// given: "a mock sync service + status/success callbacks" +// when: "runSyncToggle / runSyncRestore run for various outcomes" +// then: "consistent messages + conflict-only confirm + rollback on failure" +// observable_outcomes: +// - "enable success -> success status + ' synced (encrypted).' toast" +// - "conflict -> keep-newer status, no success toast, no rollback" +// - "nothing-to-sync -> toggle rolled back + disable() + empty hint" +// - "preEnableConfirm rejected -> toggle rolled back, enable() never called" +// - "restore imported -> success + onImported() re-render" +// determinism_controls: +// - "fully mocked service + callbacks; no DOM/network/clock" +// anti_cheat_rationale: +// prevents: ["over-mocking internal logic", "hard-coded return value"] +// relaxation: +// did_relax_any_assertion: false + +import test from "node:test"; +import { strict as assert } from "node:assert"; +import { runSyncToggle, runSyncRestore } from "../js/ui/profileModal/syncSection.js"; + +function harness(overrides = {}) { + const calls = { status: [], success: [], toggle: [], enable: 0, disable: 0, pull: 0, imported: 0 }; + const service = { + enable: async () => overrides.enableResult ?? { ok: true, accepted: 2, total: 3 }, + disable: async () => { calls.disable += 1; }, + pull: async () => { calls.pull += 1; return overrides.pullResult ?? { found: false }; }, + }; + const base = { + service, + pubkey: "f".repeat(64), + itemLabel: "wallet connection", + setStatus: (msg, tone) => calls.status.push([msg, tone]), + showSuccess: (msg) => calls.success.push(msg), + setToggle: (v) => calls.toggle.push(v), + onImported: async () => { calls.imported += 1; }, + }; + const trackedEnable = service.enable; + service.enable = async (...a) => { calls.enable += 1; return trackedEnable(...a); }; + return { calls, base }; +} + +test("enable success: success status + ' synced (encrypted).' toast", async () => { + const { calls, base } = harness(); + await runSyncToggle({ ...base, enabled: true }); + assert.ok(calls.status.some(([m, t]) => t === "success" && /Synced to 2\/3 relays/.test(m))); + assert.deepEqual(calls.success, ["Wallet connection synced (encrypted)."]); +}); + +test("conflict: keep-newer status, no success toast, no rollback", async () => { + const { calls, base } = harness({ enableResult: { ok: false, conflict: true } }); + await runSyncToggle({ ...base, enabled: true }); + assert.ok(calls.status.some(([m]) => /Kept the newer copy/.test(m))); + assert.deepEqual(calls.success, []); + assert.deepEqual(calls.toggle, []); // not rolled back + assert.equal(calls.disable, 0); +}); + +test("nothing-to-sync: rolls back toggle, disables, shows the empty hint", async () => { + const { calls, base } = harness({ enableResult: { ok: false, error: "nothing-to-sync" } }); + await runSyncToggle({ ...base, enabled: true, emptyHint: "Connect a wallet first." }); + assert.deepEqual(calls.toggle, [false]); + assert.equal(calls.disable, 1); + assert.ok(calls.status.some(([m, t]) => t === "error" && m === "Connect a wallet first.")); +}); + +test("preEnableConfirm rejected: rolls back toggle, never calls enable()", async () => { + const { calls, base } = harness(); + await runSyncToggle({ ...base, enabled: true, preEnableConfirm: async () => false }); + assert.deepEqual(calls.toggle, [false]); + assert.equal(calls.enable, 0); +}); + +test("disable: clears + 'sync turned off' toast", async () => { + const { calls, base } = harness(); + await runSyncToggle({ ...base, enabled: false }); + assert.equal(calls.disable, 1); + assert.deepEqual(calls.success, ["Wallet connection sync turned off."]); +}); + +test("restore imported: success status + toast + onImported re-render", async () => { + const { calls, base } = harness({ pullResult: { found: true, imported: true } }); + await runSyncRestore({ ...base }); + assert.ok(calls.status.some(([m, t]) => t === "success" && /Restored/.test(m))); + assert.deepEqual(calls.success, ["Wallet connection restored."]); + assert.equal(calls.imported, 1); +}); + +test("restore not found: 'No synced settings found' status", async () => { + const { calls, base } = harness({ pullResult: { found: false } }); + await runSyncRestore({ ...base }); + assert.ok(calls.status.some(([m]) => /No synced settings found/.test(m))); + assert.equal(calls.imported, 0); +}); + +test("storage vs wallet produce the SAME message shapes (just the label differs)", async () => { + const w = harness(); + await runSyncToggle({ ...w.base, itemLabel: "wallet connection", enabled: true }); + const s = harness(); + await runSyncToggle({ ...s.base, itemLabel: "storage settings", enabled: true }); + assert.deepEqual(w.calls.success, ["Wallet connection synced (encrypted)."]); + assert.deepEqual(s.calls.success, ["Storage settings synced (encrypted)."]); +});