From 3ebef19ac3a6b7ef1d562eb7a70e6061e84b7a63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Wed, 21 Oct 2026 19:42:53 +0200 Subject: [PATCH] fix(settings): one save model and one confirm per card The Telegram, ntfy, webhook and email cards saved their enable switch on tap and the rest of the form on Save; the switch is now part of the form and the one Save sends it. The display selects confirm a change with a toast. Destructive confirmations in Settings go through ConfirmButton or ConfirmDialog with the destructive style. Two structural guards hold both rules: a card may not mix instant and explicit saves, and no settings file reaches for a raw AlertDialog without a written reason. --- e2e/notification-dispatch-journey.spec.ts | 20 +- .../destructive-control-guard.test.ts | 12 +- .../success-affordance-guard.test.ts | 4 - .../__tests__/channel-form-dirty.test.ts | 77 +++++- .../__tests__/confirm-style-guard.test.ts | 77 ++++++ .../notification-channel-cards.test.tsx | 79 ++---- .../notification-channel-toggle.test.ts | 93 ------- .../__tests__/save-model-guard.test.ts | 256 ++++++++++++++++++ .../__tests__/token-revoke-labels.test.ts | 10 +- src/components/settings/advanced-section.tsx | 170 ++++-------- .../settings/ai/fallback-chain-card.tsx | 47 +--- src/components/settings/api-section.tsx | 102 ++----- src/components/settings/channel-form-dirty.ts | 46 +++- .../settings/coach-memory-section.tsx | 73 +---- .../settings/dashboard-layout-section.tsx | 1 + .../settings/date-format-select.tsx | 57 +--- src/components/settings/email-card.tsx | 83 ++---- .../settings/glucose-unit-select.tsx | 61 +---- .../settings/notification-channel-toggle.ts | 44 --- src/components/settings/ntfy-card.tsx | 84 ++---- .../security-section/passkey-list-section.tsx | 56 ++-- .../security-section/security-keys-card.tsx | 69 ++--- .../settings/security-section/totp-card.tsx | 1 + src/components/settings/sharing-section.tsx | 59 ++-- src/components/settings/sources-section.tsx | 1 + src/components/settings/telegram-card.tsx | 105 ++----- .../settings/thresholds-editor-section.tsx | 3 + .../settings/time-format-select.tsx | 57 +--- .../settings/unit-preference-select.tsx | 59 +--- src/components/settings/webhook-card.tsx | 89 ++---- .../ui/__tests__/confirm-button.test.tsx | 32 +++ src/components/ui/confirm-button.tsx | 38 ++- 32 files changed, 830 insertions(+), 1135 deletions(-) create mode 100644 src/components/settings/__tests__/confirm-style-guard.test.ts delete mode 100644 src/components/settings/__tests__/notification-channel-toggle.test.ts create mode 100644 src/components/settings/__tests__/save-model-guard.test.ts delete mode 100644 src/components/settings/notification-channel-toggle.ts diff --git a/e2e/notification-dispatch-journey.spec.ts b/e2e/notification-dispatch-journey.spec.ts index ea8b68a90..b2b0e8061 100644 --- a/e2e/notification-dispatch-journey.spec.ts +++ b/e2e/notification-dispatch-journey.spec.ts @@ -270,21 +270,6 @@ async function saveCard( return (await settled).status(); } -/** Flip a channel card's enable switch and wait for the route to answer. */ -async function toggleCard( - page: Page, - switchId: string, - endpoint: string, -): Promise { - const settled = page.waitForResponse( - (res) => - new URL(res.url()).pathname === endpoint && - res.request().method() === "PUT", - ); - await page.locator(switchId).click(); - return (await settled).status(); -} - /** * Configure both channels the way a person would: email on with a recipient, * ntfy saved and left off. Everything here happens on the settings surface. @@ -292,11 +277,10 @@ async function toggleCard( async function configureChannels(page: Page): Promise { await openChannels(page); + // The enable switch is a field of the form: it saves with the one Save. await fillField(page, "#email-recipient", EMAIL_RECIPIENT); + await page.locator("#email-toggle").click(); expect(await saveCard(page, "#email", "/api/settings/email")).toBe(200); - expect(await toggleCard(page, "#email-toggle", "/api/settings/email")).toBe( - 200, - ); await expect(page.locator("#email-toggle")).toHaveAttribute( "data-state", "checked", diff --git a/src/__tests__/destructive-control-guard.test.ts b/src/__tests__/destructive-control-guard.test.ts index 6fe80d772..75ed78be4 100644 --- a/src/__tests__/destructive-control-guard.test.ts +++ b/src/__tests__/destructive-control-guard.test.ts @@ -110,7 +110,7 @@ const REGISTRY: DestructiveEntry[] = [ destroys: "every health record on the account, or the whole account including passkeys and audit history", recovery: "permanent", - confirm: ["AlertDialog"], + confirm: ["ConfirmDialog"], }, { file: "components/settings/security-sessions-card.tsx", @@ -129,13 +129,13 @@ const REGISTRY: DestructiveEntry[] = [ file: "components/settings/security-section/passkey-list-section.tsx", destroys: "a passkey — hard-deleted, the credential cannot be re-derived", recovery: "permanent", - confirm: ["AlertDialog"], + confirm: ["ConfirmButton"], }, { file: "components/settings/security-section/security-keys-card.tsx", destroys: "a WebAuthn second factor", recovery: "permanent", - confirm: ["AlertDialog"], + confirm: ["ConfirmButton"], }, { file: "components/settings/ai/remove-saved-key-button.tsx", @@ -156,7 +156,7 @@ const REGISTRY: DestructiveEntry[] = [ file: "components/settings/api-section.tsx", destroys: "an API token — revoked, and the secret is never shown again", recovery: "permanent", - confirm: ["AlertDialog"], + confirm: ["ConfirmButton"], }, { file: "components/settings/mcp-section.tsx", @@ -168,7 +168,7 @@ const REGISTRY: DestructiveEntry[] = [ file: "components/settings/sharing-section.tsx", destroys: "a clinician share link — the URL stays dead once revoked", recovery: "permanent", - confirm: ["AlertDialog"], + confirm: ["ConfirmButton"], }, { file: "components/settings/account-section/avatar-section.tsx", @@ -214,7 +214,7 @@ const REGISTRY: DestructiveEntry[] = [ file: "components/settings/coach-memory-section.tsx", destroys: "every remembered Coach fact at once", recovery: "tombstoned-no-restore", - confirm: ["AlertDialog"], + confirm: ["ConfirmButton"], }, { file: "components/insights/coach-panel/use-coach-facts.ts", diff --git a/src/__tests__/success-affordance-guard.test.ts b/src/__tests__/success-affordance-guard.test.ts index a41563ed7..2b78f8702 100644 --- a/src/__tests__/success-affordance-guard.test.ts +++ b/src/__tests__/success-affordance-guard.test.ts @@ -348,7 +348,6 @@ const PINNED_AFFORDANCES: Record< "src/components/settings/dashboard-layout-section.tsx": { "toast.success": 2, }, - "src/components/settings/email-card.tsx": { "text-success": 1 }, "src/components/settings/environment-section.tsx": { "toast.success": 4 }, "src/components/settings/insights-pill-order-section.tsx": { "toast.success": 1, @@ -389,7 +388,6 @@ const PINNED_AFFORDANCES: Record< CheckCircle2: 2, "text-success": 1, }, - "src/components/settings/ntfy-card.tsx": { "text-success": 1 }, // The one that used to sit inside `account-section/index.tsx` — the card // moved to Settings → Security, the line moved with it. "src/components/settings/password-card.tsx": { "text-success": 1 }, @@ -397,7 +395,6 @@ const PINNED_AFFORDANCES: Record< "src/components/settings/share-link-create-form.tsx": { "text-success": 3 }, "src/components/settings/sharing-section.tsx": { "text-success": 1 }, "src/components/settings/sources-section.tsx": { "toast.success": 2 }, - "src/components/settings/telegram-card.tsx": { "text-success": 1 }, "src/components/settings/test-connection-button.tsx": { CheckCircle2: 2, "text-success": 1, @@ -407,7 +404,6 @@ const PINNED_AFFORDANCES: Record< }, "src/components/settings/trusted-devices-card.tsx": { "text-success": 1 }, "src/components/settings/web-push-card.tsx": { "text-success": 1 }, - "src/components/settings/webhook-card.tsx": { "text-success": 1 }, "src/components/targets/target-edit-sheet.tsx": { "toast.success": 2 }, "src/components/ui/password-strength.tsx": { "text-success": 2 }, // The success tone of the grade chip (a high confidence, a score that went diff --git a/src/components/settings/__tests__/channel-form-dirty.test.ts b/src/components/settings/__tests__/channel-form-dirty.test.ts index 63e57592c..cb20527dc 100644 --- a/src/components/settings/__tests__/channel-form-dirty.test.ts +++ b/src/components/settings/__tests__/channel-form-dirty.test.ts @@ -21,6 +21,7 @@ describe("channel form dirty state", () => { headerName: "X-Gotify-Key", headerValue: "", format: "gotify" as const, + enabled: false, }; it("webhook: a form that matches the saved config is clean", () => { @@ -56,7 +57,12 @@ describe("channel form dirty state", () => { it("ntfy: server, topic and a typed auth token count", () => { const saved = { serverUrl: "https://ntfy.sh", topic: "t" }; - const form = { serverUrl: "https://ntfy.sh", topic: "t", authToken: "" }; + const form = { + serverUrl: "https://ntfy.sh", + topic: "t", + authToken: "", + enabled: false, + }; expect(isNtfyFormDirty(form, saved)).toBe(false); expect(isNtfyFormDirty({ ...form, topic: "u" }, saved)).toBe(true); expect(isNtfyFormDirty({ ...form, serverUrl: "https://x" }, saved)).toBe( @@ -67,39 +73,86 @@ describe("channel form dirty state", () => { it("email: the recipient counts", () => { expect( - isEmailFormDirty({ recipient: "a@x.io" }, { recipient: "a@x.io" }), + isEmailFormDirty( + { recipient: "a@x.io", enabled: false }, + { recipient: "a@x.io" }, + ), ).toBe(false); expect( - isEmailFormDirty({ recipient: "b@x.io" }, { recipient: "a@x.io" }), + isEmailFormDirty( + { recipient: "b@x.io", enabled: false }, + { recipient: "a@x.io" }, + ), ).toBe(true); }); it("telegram: a typed bot token or a changed chat id counts; whitespace alone does not", () => { expect( - isTelegramFormDirty({ botToken: "", chatId: "1" }, { chatId: "1" }), - ).toBe(false); - expect( - isTelegramFormDirty({ botToken: " ", chatId: "" }, { chatId: null }), + isTelegramFormDirty( + { botToken: "", chatId: "1", enabled: false }, + { chatId: "1" }, + ), ).toBe(false); expect( isTelegramFormDirty( - { botToken: "123:abc", chatId: "1" }, + { botToken: " ", chatId: "", enabled: false }, + { chatId: null }, + ), + ).toBe(false); + expect( + isTelegramFormDirty( + { botToken: "123:abc", chatId: "1", enabled: false }, { chatId: "1" }, ), ).toBe(true); expect( - isTelegramFormDirty({ botToken: "", chatId: "2" }, { chatId: "1" }), + isTelegramFormDirty( + { botToken: "", chatId: "2", enabled: false }, + { chatId: "1" }, + ), ).toBe(true); }); + it("a flipped enable switch is a change on every channel, since it saves with the form", () => { + expect( + isWebhookFormDirty({ ...webhookForm, enabled: true }, webhookSaved), + ).toBe(true); + expect( + isNtfyFormDirty( + { serverUrl: "s", topic: "t", authToken: "", enabled: true }, + { serverUrl: "s", topic: "t", enabled: false }, + ), + ).toBe(true); + expect( + isEmailFormDirty( + { recipient: "a@x.io", enabled: false }, + { recipient: "a@x.io", enabled: true }, + ), + ).toBe(true); + expect( + isTelegramFormDirty( + { botToken: "", chatId: "1", enabled: true }, + { chatId: "1", enabled: true }, + ), + ).toBe(false); + }); + it("nothing loaded yet is never a change", () => { expect(isWebhookFormDirty(webhookForm, undefined)).toBe(false); expect( - isNtfyFormDirty({ serverUrl: "", topic: "", authToken: "x" }, undefined), + isNtfyFormDirty( + { serverUrl: "", topic: "", authToken: "x", enabled: true }, + undefined, + ), ).toBe(false); - expect(isEmailFormDirty({ recipient: "x" }, undefined)).toBe(false); - expect(isTelegramFormDirty({ botToken: "x", chatId: "" }, undefined)).toBe( + expect(isEmailFormDirty({ recipient: "x", enabled: true }, undefined)).toBe( false, ); + expect( + isTelegramFormDirty( + { botToken: "x", chatId: "", enabled: true }, + undefined, + ), + ).toBe(false); }); }); diff --git a/src/components/settings/__tests__/confirm-style-guard.test.ts b/src/components/settings/__tests__/confirm-style-guard.test.ts new file mode 100644 index 000000000..a83d4f0c3 --- /dev/null +++ b/src/components/settings/__tests__/confirm-style-guard.test.ts @@ -0,0 +1,77 @@ +/** + * One confirm style across Settings. + * + * A destructive act (delete, revoke, remove, reset, sign out) confirms through + * `ConfirmButton` / `ConfirmDialog` (`src/components/ui/confirm-button.tsx`), + * which owns the dialog's anatomy and wears the destructive style when the + * caller says the act is destructive. A hand-built `AlertDialog` is how the + * same question ended up styled three different ways on neighbouring cards. + * + * So no file under `src/components/settings` imports the raw `AlertDialog` + * unless it is listed below with the reason the primitive does not fit. + */ +import { readdirSync, readFileSync, statSync } from "node:fs"; +import { join, relative } from "node:path"; + +import { describe, expect, it } from "vitest"; + +const ROOT = join(process.cwd(), "src", "components", "settings"); + +const ALLOWED: Readonly> = { + "security-section/existing-factor-reauth-dialog.tsx": + "a re-authentication form, not a destructive confirm", + "security-section/passkey-list-section.tsx": + "the add-passkey re-authentication form; the delete confirm uses ConfirmButton", + "security-section/totp-card.tsx": + "the disable confirm asks for a current code and stays open until the server answers; both actions wear the destructive style", + "mcp-section.tsx": + "the connection and token revoke confirms; the MCP card is reshaped separately", +}; + +/** Owned by the integration cards, which follow their own card kit. */ +const ALLOWED_PREFIXES: readonly string[] = ["integrations/"]; + +function walk(dir: string): string[] { + return readdirSync(dir).flatMap((name) => { + const path = join(dir, name); + if (statSync(path).isDirectory()) { + return name === "__tests__" ? [] : walk(path); + } + return /\.tsx?$/.test(name) ? [path] : []; + }); +} + +const IMPORTS_ALERT_DIALOG = /from\s+["']@\/components\/ui\/alert-dialog["']/; + +describe("settings confirm style", () => { + const files = walk(ROOT); + + it("scans the settings tree", () => { + expect(files.length).toBeGreaterThan(50); + }); + + it("imports the raw AlertDialog only where the primitive does not fit", () => { + const offenders = files + .map((path) => relative(ROOT, path)) + .filter((rel) => + IMPORTS_ALERT_DIALOG.test(readFileSync(join(ROOT, rel), "utf8")), + ) + .filter( + (rel) => + !(rel in ALLOWED) && + !ALLOWED_PREFIXES.some((prefix) => rel.startsWith(prefix)), + ); + expect( + offenders, + "Confirm a destructive act through ConfirmButton / ConfirmDialog, or list the file here with the reason", + ).toEqual([]); + }); + + it("lists no file that no longer needs the exemption", () => { + const stale = Object.keys(ALLOWED).filter( + (rel) => + !IMPORTS_ALERT_DIALOG.test(readFileSync(join(ROOT, rel), "utf8")), + ); + expect(stale).toEqual([]); + }); +}); diff --git a/src/components/settings/__tests__/notification-channel-cards.test.tsx b/src/components/settings/__tests__/notification-channel-cards.test.tsx index 85861ac18..26a2d5b2a 100644 --- a/src/components/settings/__tests__/notification-channel-cards.test.tsx +++ b/src/components/settings/__tests__/notification-channel-cards.test.tsx @@ -121,60 +121,27 @@ describe("notification channel cards", () => { ); it.each([ - ["webhook", , 1], - ["ntfy", , 1], - ["email", , 1], - ["telegram", , 0], + ["webhook", ], + ["ntfy", ], + ["email", ], + ["telegram", ], ] as const)( - "%s switch invokes the enable-only mutation rather than the full save", - async (channel, card, toggleMutationIndex) => { - render(card); - - await mocks.mutationOptions[toggleMutationIndex]!.mutationFn(true); - - expect(mocks.apiFetchRaw).toHaveBeenCalledTimes(1); - expect(mocks.apiFetchRaw).toHaveBeenCalledWith( - `/api/settings/${channel}`, - { - method: "PUT", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ enabled: true }), - }, - ); - }, - ); - - it.each([ - [, 1, ["settings", "webhook"]], - [, 1, ["settings", "ntfy"]], - [, 1, ["settings", "email"]], - [ - , - 0, - ["telegram", "settings"], - ], - ] as const)( - "rolls each visible switch back to its previous server state on error", - async (card, toggleMutationIndex, queryKey) => { - const previous = { enabled: false, persistedConfig: "unchanged" }; - mocks.queryClient.getQueryData.mockReturnValue(previous); - render(card); - const toggle = mocks.mutationOptions[toggleMutationIndex]!; - - const context = await toggle.onMutate!(true); - const optimisticUpdater = mocks.queryClient.setQueryData.mock - .calls[0]![1] as (current: typeof previous) => typeof previous; - expect(optimisticUpdater(previous)).toEqual({ - ...previous, - enabled: true, - }); - - toggle.onError!(new Error("save failed"), true, context); - - expect(mocks.queryClient.setQueryData).toHaveBeenLastCalledWith( - queryKey, - previous, - ); + "%s enable switch is a form field: it writes nothing on its own", + async (_channel, card) => { + const html = render(card); + // Rendering the switch writes nothing, and the card mounts no + // switch-only mutation: every write it can make is the full form. + expect(html).toContain('role="switch"'); + expect(mocks.apiFetchRaw).not.toHaveBeenCalled(); + for (const mutation of mocks.mutationOptions) { + await mutation.mutationFn(true); + } + for (const [, init] of mocks.apiFetchRaw.mock.calls as Array< + [string, { body: string }] + >) { + const body = JSON.parse(init.body) as Record; + expect(Object.keys(body).length).toBeGreaterThan(1); + } }, ); @@ -185,9 +152,9 @@ describe("notification channel cards", () => { expect(html).toMatch(/ ))} -

- {msg ?? t("settings.dateFormat.description")} +

+ {t("settings.dateFormat.description")}

); diff --git a/src/components/settings/email-card.tsx b/src/components/settings/email-card.tsx index 5cd80fe91..0ea1b1657 100644 --- a/src/components/settings/email-card.tsx +++ b/src/components/settings/email-card.tsx @@ -13,11 +13,8 @@ import { SettingsCard } from "@/components/settings/settings-card"; import { SettingsCardHeader } from "@/components/settings/_card-header"; import { TestConnectionButton } from "@/components/settings/test-connection-button"; import { isEmailFormDirty } from "@/components/settings/channel-form-dirty"; -import { - optimisticallySetChannelEnabled, - persistChannelEnabled, - rollbackChannelEnabled, -} from "@/components/settings/notification-channel-toggle"; +import { toastWrittenOutcome } from "@/components/outcome/outcome-toast"; +import { showErrorToast } from "@/lib/errors/error-toast"; import { useTranslations } from "@/lib/i18n/context"; import { queryKeys } from "@/lib/query-keys"; import { apiFetchRaw, apiGet } from "@/lib/api/api-fetch"; @@ -33,10 +30,9 @@ export function EmailCard({ isAuthenticated }: { isAuthenticated: boolean }) { const { t } = useTranslations(); const queryClient = useQueryClient(); const [recipient, setRecipient] = useState(""); - const [saveMsg, setSaveMsg] = useState(null); - const [saveMsgType, setSaveMsgType] = useState<"success" | "error" | null>( - null, - ); + // The enable switch is a draft field: it saves with the form's one Save. + const [enabled, setEnabled] = useState(false); + const [saveError, setSaveError] = useState(null); const { data: settings } = useQuery({ queryKey: queryKeys.settingsEmail(), @@ -47,16 +43,19 @@ export function EmailCard({ isAuthenticated }: { isAuthenticated: boolean }) { }); // React-recommended sync-from-server pattern (no setState-in-effect). - const settingsKey = settings ? settings.recipient : null; + const settingsKey = settings + ? `${settings.recipient}|${settings.enabled}` + : null; const [seededKey, setSeededKey] = useState(null); if (settingsKey !== null && settingsKey !== seededKey) { setSeededKey(settingsKey); setRecipient(settings!.recipient); + setEnabled(settings!.enabled); } - const dirty = isEmailFormDirty({ recipient }, settings); + const dirty = isEmailFormDirty({ recipient, enabled }, settings); const save = useMutation({ - mutationFn: async (enabled: boolean) => { + mutationFn: async () => { const res = await apiFetchRaw("/api/settings/email", { method: "PUT", headers: { "Content-Type": "application/json" }, @@ -69,45 +68,12 @@ export function EmailCard({ isAuthenticated }: { isAuthenticated: boolean }) { }, onSuccess: () => { queryClient.invalidateQueries({ queryKey: queryKeys.settingsEmail() }); - setSaveMsg(t("settings.saved")); - setSaveMsgType("success"); + setSaveError(null); + toastWrittenOutcome("success", t("settings.saved")); }, onError: (err: Error) => { - setSaveMsg(err.message); - setSaveMsgType("error"); - }, - }); - - const toggleEnabled = useMutation< - void, - Error, - boolean, - EmailSettings | undefined - >({ - mutationFn: (enabled) => - persistChannelEnabled("/api/settings/email", enabled, t("common.error")), - onMutate: async (enabled) => { - const queryKey = queryKeys.settingsEmail(); - await queryClient.cancelQueries({ queryKey }); - setSaveMsg(null); - setSaveMsgType(null); - return optimisticallySetChannelEnabled( - queryClient, - queryKey, - enabled, - ); - }, - onSuccess: () => { - setSaveMsg(t("settings.saved")); - setSaveMsgType("success"); - }, - onError: (err, _enabled, previous) => { - rollbackChannelEnabled(queryClient, queryKeys.settingsEmail(), previous); - setSaveMsg(err.message); - setSaveMsgType("error"); - }, - onSettled: () => { - queryClient.invalidateQueries({ queryKey: queryKeys.settingsEmail() }); + setSaveError(err.message); + showErrorToast(err.message, { cause: err }); }, }); @@ -128,16 +94,16 @@ export function EmailCard({ isAuthenticated }: { isAuthenticated: boolean }) { toggleEnabled.mutate(checked)} - disabled={save.isPending || toggleEnabled.isPending} + checked={enabled} + onCheckedChange={setEnabled} + disabled={save.isPending} />
{ e.preventDefault(); - save.mutate(settings?.enabled ?? false); + save.mutate(); }} className="space-y-4" > @@ -156,12 +122,9 @@ export function EmailCard({ isAuthenticated }: { isAuthenticated: boolean }) { /> - {saveMsg && (saveMsgType === "error" || !dirty) && ( -

- {saveMsg} + {saveError && ( +

+ {saveError}

)} @@ -173,7 +136,7 @@ export function EmailCard({ isAuthenticated }: { isAuthenticated: boolean }) { /> - - - - - - - - {t("settings.deletePasskey")} - - - {t("settings.deletePasskeyDescription")} - - - - - {t("common.cancel")} - - remove.mutate(pk.id)} - > - {remove.isPending && ( - - )} - {t("common.delete")} - - - - + remove.mutate(pk.id)} + pending={remove.isPending && remove.variables === pk.id} + disabled={remove.isPending} + size="icon" + variant="ghost" + className="text-muted-foreground hover:text-foreground min-h-11 min-w-11 sm:h-8 sm:min-h-0 sm:w-8 sm:min-w-0" + icon={} + destructive + /> )} diff --git a/src/components/settings/security-section/security-keys-card.tsx b/src/components/settings/security-section/security-keys-card.tsx index ce4b99415..8bb5c0a8a 100644 --- a/src/components/settings/security-section/security-keys-card.tsx +++ b/src/components/settings/security-section/security-keys-card.tsx @@ -13,18 +13,8 @@ import { X, } from "lucide-react"; -import { - AlertDialog, - AlertDialogAction, - AlertDialogCancel, - AlertDialogContent, - AlertDialogDescription, - AlertDialogFooter, - AlertDialogHeader, - AlertDialogTitle, - AlertDialogTrigger, -} from "@/components/ui/alert-dialog"; import { Button } from "@/components/ui/button"; +import { ConfirmButton } from "@/components/ui/confirm-button"; import { Input } from "@/components/ui/input"; import { EmptyState } from "@/components/ui/empty-state"; import { SettingsCard } from "@/components/settings/settings-card"; @@ -308,46 +298,23 @@ export function SecurityKeysCard({ > - - - - - - - - {t("settings.security.keys.remove")} - - - {t("settings.security.keys.removeConfirm")} - - - - - {t("common.cancel")} - - remove.mutate(key.id)} - > - {remove.isPending && ( - - )} - {t("common.delete")} - - - - + remove.mutate(key.id)} + pending={ + remove.isPending && remove.variables === key.id + } + disabled={remove.isPending} + size="icon" + variant="ghost" + className="text-muted-foreground hover:text-foreground min-h-11 min-w-11 sm:h-8 sm:min-h-0 sm:w-8 sm:min-w-0" + icon={} + destructive + /> )} diff --git a/src/components/settings/security-section/totp-card.tsx b/src/components/settings/security-section/totp-card.tsx index 17e62753f..2acac8456 100644 --- a/src/components/settings/security-section/totp-card.tsx +++ b/src/components/settings/security-section/totp-card.tsx @@ -498,6 +498,7 @@ export function TotpCard({ {t("common.cancel")} - - - - - - - {t("settings.sharing.revoke")} - - - {t("settings.sharing.revokeDescription")} - - - - - {t("common.cancel")} - - revokeMutation.mutate(link.id)} - > - {t("settings.sharing.revoke")} - - - - + revokeMutation.mutate(link.id)} + pending={ + revokeMutation.isPending && + revokeMutation.variables === link.id + } + size="sm" + className="ml-auto flex min-h-11 w-fit sm:min-h-9" + icon={} + destructive + /> ))} diff --git a/src/components/settings/sources-section.tsx b/src/components/settings/sources-section.tsx index 1860af9dc..4846c99eb 100644 --- a/src/components/settings/sources-section.tsx +++ b/src/components/settings/sources-section.tsx @@ -697,6 +697,7 @@ export function SourcesSection() { disabled={saveMutation.isPending} pending={resetMutation.isPending} onConfirm={() => resetMutation.mutate()} + destructive /> {dirty && priority ? ( <> diff --git a/src/components/settings/telegram-card.tsx b/src/components/settings/telegram-card.tsx index 3c685ce6b..551974655 100644 --- a/src/components/settings/telegram-card.tsx +++ b/src/components/settings/telegram-card.tsx @@ -1,7 +1,7 @@ "use client"; import { useState } from "react"; -import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; +import { useQuery, useQueryClient } from "@tanstack/react-query"; import { Info, Loader2, MessageCircle, Save } from "lucide-react"; import { Button } from "@/components/ui/button"; @@ -15,11 +15,8 @@ import { SettingsCard } from "@/components/settings/settings-card"; import { SettingsCardHeader } from "@/components/settings/_card-header"; import { TestConnectionButton } from "@/components/settings/test-connection-button"; import { isTelegramFormDirty } from "@/components/settings/channel-form-dirty"; -import { - optimisticallySetChannelEnabled, - persistChannelEnabled, - rollbackChannelEnabled, -} from "@/components/settings/notification-channel-toggle"; +import { toastWrittenOutcome } from "@/components/outcome/outcome-toast"; +import { showErrorToast } from "@/lib/errors/error-toast"; import { useTranslations } from "@/lib/i18n/context"; import { queryKeys } from "@/lib/query-keys"; import { apiFetchRaw, apiGet } from "@/lib/api/api-fetch"; @@ -39,9 +36,10 @@ export function TelegramCard({ const queryClient = useQueryClient(); const [botToken, setBotToken] = useState(""); const [chatId, setChatId] = useState(""); + // The enable switch is a draft field: it saves with the form's one Save. + const [enabled, setEnabled] = useState(false); const [saving, setSaving] = useState(false); - const [msg, setMsg] = useState(null); - const [msgType, setMsgType] = useState<"success" | "error" | null>(null); + const [error, setError] = useState(null); const { data: settings } = useQuery({ queryKey: queryKeys.telegramSettings(), @@ -52,67 +50,25 @@ export function TelegramCard({ }); // Sync from server using the React-recommended "previous-payload-id" - // pattern instead of setState-in-effect. This avoids the strict - // `react-hooks/set-state-in-effect` lint failure that the legacy file - // disabled at module scope. - const settingsKey = settings ? (settings.chatId ?? "") : null; + // pattern instead of setState-in-effect. Reseeds after every save, so the + // switch reads what the server stored. + const settingsKey = settings + ? `${settings.chatId ?? ""}|${settings.enabled}` + : null; const [seededKey, setSeededKey] = useState(null); if (settingsKey && settingsKey !== seededKey) { setSeededKey(settingsKey); if (settings!.chatId) setChatId(settings!.chatId); + setEnabled(settings!.enabled); } - const dirty = isTelegramFormDirty({ botToken, chatId }, settings); - - const toggleEnabled = useMutation< - void, - Error, - boolean, - TelegramSettings | undefined - >({ - mutationFn: (enabled) => - persistChannelEnabled( - "/api/settings/telegram", - enabled, - t("settings.savingError"), - ), - onMutate: async (enabled) => { - const queryKey = queryKeys.telegramSettings(); - await queryClient.cancelQueries({ queryKey }); - setMsg(null); - setMsgType(null); - return optimisticallySetChannelEnabled( - queryClient, - queryKey, - enabled, - ); - }, - onSuccess: () => { - setMsg(t("settings.saved")); - setMsgType("success"); - }, - onError: (err, _enabled, previous) => { - rollbackChannelEnabled( - queryClient, - queryKeys.telegramSettings(), - previous, - ); - setMsg(err.message); - setMsgType("error"); - }, - onSettled: () => { - queryClient.invalidateQueries({ queryKey: queryKeys.telegramSettings() }); - }, - }); + const dirty = isTelegramFormDirty({ botToken, chatId, enabled }, settings); async function handleSave(e: React.FormEvent) { e.preventDefault(); setSaving(true); - setMsg(null); - setMsgType(null); + setError(null); - const body: Record = { - enabled: settings?.enabled ?? false, - }; + const body: Record = { enabled }; if (botToken.trim()) body.botToken = botToken.trim(); if (chatId !== (settings?.chatId ?? "")) body.chatId = chatId; @@ -126,16 +82,16 @@ export function TelegramCard({ }).catch(() => null); if (res?.ok) { - setMsg(t("settings.saved")); - setMsgType("success"); + toastWrittenOutcome("success", t("settings.saved")); setBotToken(""); queryClient.invalidateQueries({ queryKey: queryKeys.telegram() }); } else { const json = (await res?.json().catch(() => null)) as { error?: string; } | null; - setMsg(json?.error || t("settings.savingError")); - setMsgType("error"); + const message = json?.error || t("settings.savingError"); + setError(message); + showErrorToast(message); } } finally { setSaving(false); @@ -192,9 +148,9 @@ export function TelegramCard({
toggleEnabled.mutate(checked)} - disabled={saving || toggleEnabled.isPending} + checked={enabled} + onCheckedChange={setEnabled} + disabled={saving} />
diff --git a/src/components/settings/time-format-select.tsx b/src/components/settings/time-format-select.tsx index d8c0d823e..528c5e37d 100644 --- a/src/components/settings/time-format-select.tsx +++ b/src/components/settings/time-format-select.tsx @@ -1,8 +1,11 @@ "use client"; -import { useEffect, useRef, useState } from "react"; +import { useState } from "react"; import { useMutation, useQueryClient } from "@tanstack/react-query"; +import { toastWrittenOutcome } from "@/components/outcome/outcome-toast"; +import { showErrorToast } from "@/lib/errors/error-toast"; + import { Label } from "@/components/ui/label"; import { NativeSelect } from "@/components/ui/native-select"; import { useAuth } from "@/hooks/use-auth"; @@ -39,30 +42,6 @@ export function TimeFormatSelect({ const [optimistic, setOptimistic] = useState( null, ); - const [msg, setMsg] = useState(null); - const [msgType, setMsgType] = useState<"success" | "error" | null>(null); - const clearTimerRef = useRef | null>(null); - - useEffect(() => { - return () => { - if (clearTimerRef.current !== null) { - clearTimeout(clearTimerRef.current); - clearTimerRef.current = null; - } - }; - }, []); - - function scheduleClear() { - if (clearTimerRef.current !== null) { - clearTimeout(clearTimerRef.current); - } - clearTimerRef.current = setTimeout(() => { - clearTimerRef.current = null; - setMsg(null); - setMsgType(null); - }, 3000); - } - const value: TimeFormatPreference = optimistic ?? user?.timeFormat ?? "AUTO"; const mutation = useMutation({ @@ -84,31 +63,19 @@ export function TimeFormatSelect({ // Repaint every formatter consumer right away; the /me refetch below // re-asserts the same value once it settles. storeTimeFormat(next); - setMsg(t("settings.timeFormat.saved")); - setMsgType("success"); + toastWrittenOutcome("success", t("settings.timeFormat.saved")); queryClient.invalidateQueries({ queryKey: queryKeys.authMe() }); setOptimistic(null); - scheduleClear(); }, onError: (err) => { setOptimistic(null); - setMsg( - err instanceof Error ? err.message : t("settings.timeFormat.saveError"), - ); - setMsgType("error"); - scheduleClear(); + showErrorToast(t("settings.timeFormat.saveError"), { cause: err }); }, }); function handleSelect(next: TimeFormatPreference) { if (next === value || mutation.isPending || !isAuthenticated) return; setOptimistic(next); - setMsg(null); - setMsgType(null); - if (clearTimerRef.current !== null) { - clearTimeout(clearTimerRef.current); - clearTimerRef.current = null; - } mutation.mutate(next); } @@ -126,16 +93,8 @@ export function TimeFormatSelect({ -

- {msg ?? t("settings.timeFormat.description")} +

+ {t("settings.timeFormat.description")}

); diff --git a/src/components/settings/unit-preference-select.tsx b/src/components/settings/unit-preference-select.tsx index 43a435715..0ba6d4712 100644 --- a/src/components/settings/unit-preference-select.tsx +++ b/src/components/settings/unit-preference-select.tsx @@ -1,8 +1,11 @@ "use client"; -import { useEffect, useRef, useState } from "react"; +import { useState } from "react"; import { useMutation, useQueryClient } from "@tanstack/react-query"; +import { toastWrittenOutcome } from "@/components/outcome/outcome-toast"; +import { showErrorToast } from "@/lib/errors/error-toast"; + import { Label } from "@/components/ui/label"; import { NativeSelect } from "@/components/ui/native-select"; import { useAuth } from "@/hooks/use-auth"; @@ -36,30 +39,6 @@ export function UnitPreferenceSelect({ const { user } = useAuth(); const [optimistic, setOptimistic] = useState(null); - const [msg, setMsg] = useState(null); - const [msgType, setMsgType] = useState<"success" | "error" | null>(null); - const clearTimerRef = useRef | null>(null); - - useEffect(() => { - return () => { - if (clearTimerRef.current !== null) { - clearTimeout(clearTimerRef.current); - clearTimerRef.current = null; - } - }; - }, []); - - function scheduleClear() { - if (clearTimerRef.current !== null) { - clearTimeout(clearTimerRef.current); - } - clearTimerRef.current = setTimeout(() => { - clearTimerRef.current = null; - setMsg(null); - setMsgType(null); - }, 3000); - } - const value: UnitPreference = optimistic ?? user?.unitPreference ?? "metric"; const mutation = useMutation({ @@ -78,36 +57,22 @@ export function UnitPreferenceSelect({ return next; }, onSuccess: () => { - setMsg(t("settings.dashboard.units.saved")); - setMsgType("success"); + toastWrittenOutcome("success", t("settings.dashboard.units.saved")); queryClient.invalidateQueries({ queryKey: queryKeys.authMe() }); queryClient.invalidateQueries({ queryKey: queryKeys.userUnitPreference(), }); setOptimistic(null); - scheduleClear(); }, onError: (err) => { setOptimistic(null); - setMsg( - err instanceof Error - ? err.message - : t("settings.dashboard.units.saveError"), - ); - setMsgType("error"); - scheduleClear(); + showErrorToast(t("settings.dashboard.units.saveError"), { cause: err }); }, }); function handleSelect(next: UnitPreference) { if (next === value || mutation.isPending || !isAuthenticated) return; setOptimistic(next); - setMsg(null); - setMsgType(null); - if (clearTimerRef.current !== null) { - clearTimeout(clearTimerRef.current); - clearTimerRef.current = null; - } mutation.mutate(next); } @@ -126,16 +91,8 @@ export function UnitPreferenceSelect({ {t("settings.dashboard.units.imperial")} -

- {msg ?? t("settings.dashboard.units.description")} +

+ {t("settings.dashboard.units.description")}

); diff --git a/src/components/settings/webhook-card.tsx b/src/components/settings/webhook-card.tsx index f09528c7e..de11154fc 100644 --- a/src/components/settings/webhook-card.tsx +++ b/src/components/settings/webhook-card.tsx @@ -15,11 +15,8 @@ import { SettingsCard } from "@/components/settings/settings-card"; import { SettingsCardHeader } from "@/components/settings/_card-header"; import { TestConnectionButton } from "@/components/settings/test-connection-button"; import { isWebhookFormDirty } from "@/components/settings/channel-form-dirty"; -import { - optimisticallySetChannelEnabled, - persistChannelEnabled, - rollbackChannelEnabled, -} from "@/components/settings/notification-channel-toggle"; +import { toastWrittenOutcome } from "@/components/outcome/outcome-toast"; +import { showErrorToast } from "@/lib/errors/error-toast"; import { useTranslations } from "@/lib/i18n/context"; import { queryKeys } from "@/lib/query-keys"; import { apiFetchRaw, apiGet } from "@/lib/api/api-fetch"; @@ -41,10 +38,9 @@ export function WebhookCard({ isAuthenticated }: { isAuthenticated: boolean }) { const [headerName, setHeaderName] = useState(""); const [headerValue, setHeaderValue] = useState(""); const [format, setFormat] = useState("generic"); - const [saveMsg, setSaveMsg] = useState(null); - const [saveMsgType, setSaveMsgType] = useState<"success" | "error" | null>( - null, - ); + // The enable switch is a draft field: it saves with the form's one Save. + const [enabled, setEnabled] = useState(false); + const [saveError, setSaveError] = useState(null); const { data: settings } = useQuery({ queryKey: queryKeys.settingsWebhook(), @@ -56,7 +52,7 @@ export function WebhookCard({ isAuthenticated }: { isAuthenticated: boolean }) { // React-recommended sync-from-server pattern (no setState-in-effect). const settingsKey = settings - ? `${settings.url}|${settings.headerName}|${settings.format ?? "generic"}` + ? `${settings.url}|${settings.headerName}|${settings.format ?? "generic"}|${settings.enabled}` : null; const [seededKey, setSeededKey] = useState(null); if (settingsKey && settingsKey !== seededKey) { @@ -64,14 +60,15 @@ export function WebhookCard({ isAuthenticated }: { isAuthenticated: boolean }) { setUrl(settings!.url); setHeaderName(settings!.headerName); setFormat(settings!.format === "gotify" ? "gotify" : "generic"); + setEnabled(settings!.enabled); } const dirty = isWebhookFormDirty( - { url, headerName, headerValue, format }, + { url, headerName, headerValue, format, enabled }, settings, ); const save = useMutation({ - mutationFn: async (enabled: boolean) => { + mutationFn: async () => { const res = await apiFetchRaw("/api/settings/webhook", { method: "PUT", headers: { "Content-Type": "application/json" }, @@ -102,54 +99,13 @@ export function WebhookCard({ isAuthenticated }: { isAuthenticated: boolean }) { }, onSuccess: () => { queryClient.invalidateQueries({ queryKey: queryKeys.settingsWebhook() }); - setSaveMsg(t("settings.saved")); - setSaveMsgType("success"); + setSaveError(null); + toastWrittenOutcome("success", t("settings.saved")); setHeaderValue(""); }, onError: (err: Error) => { - setSaveMsg(err.message); - setSaveMsgType("error"); - }, - }); - - const toggleEnabled = useMutation< - void, - Error, - boolean, - WebhookSettings | undefined - >({ - mutationFn: (enabled) => - persistChannelEnabled( - "/api/settings/webhook", - enabled, - t("common.error"), - ), - onMutate: async (enabled) => { - const queryKey = queryKeys.settingsWebhook(); - await queryClient.cancelQueries({ queryKey }); - setSaveMsg(null); - setSaveMsgType(null); - return optimisticallySetChannelEnabled( - queryClient, - queryKey, - enabled, - ); - }, - onSuccess: () => { - setSaveMsg(t("settings.saved")); - setSaveMsgType("success"); - }, - onError: (err, _enabled, previous) => { - rollbackChannelEnabled( - queryClient, - queryKeys.settingsWebhook(), - previous, - ); - setSaveMsg(err.message); - setSaveMsgType("error"); - }, - onSettled: () => { - queryClient.invalidateQueries({ queryKey: queryKeys.settingsWebhook() }); + setSaveError(err.message); + showErrorToast(err.message, { cause: err }); }, }); @@ -166,16 +122,16 @@ export function WebhookCard({ isAuthenticated }: { isAuthenticated: boolean }) { toggleEnabled.mutate(checked)} - disabled={save.isPending || toggleEnabled.isPending} + checked={enabled} + onCheckedChange={setEnabled} + disabled={save.isPending} /> { e.preventDefault(); - save.mutate(settings?.enabled ?? false); + save.mutate(); }} className="space-y-4" > @@ -244,12 +200,9 @@ export function WebhookCard({ isAuthenticated }: { isAuthenticated: boolean }) { - {saveMsg && (saveMsgType === "error" || !dirty) && ( -

- {saveMsg} + {saveError && ( +

+ {saveError}

)} @@ -261,7 +214,7 @@ export function WebhookCard({ isAuthenticated }: { isAuthenticated: boolean }) { />