From 9cfc72fe1fac02c50e8903538bbc9a3a4367dbd5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Wed, 21 Oct 2026 21:58:22 +0200 Subject: [PATCH] fix(settings): channel switches act on tap again, beside their form The Telegram, ntfy, webhook and email cards had folded the enable switch into the form, so turning a channel off took a Save as well. The switch saves on tap again, optimistically with a rollback, and sits outside the form; the form's Save covers the channel's fields and sends the stored enable state unchanged. The save-model guard now splits a card into save units, each form and the rest of the card, and refuses a unit that mixes both models: an instant switch inside the form its Save submits, or a Save outside any form next to one, still fails. A form shows its refusal inline only, no longer as a toast on top. --- e2e/notification-dispatch-journey.spec.ts | 20 +++- .../__tests__/channel-form-dirty.test.ts | 77 +++------------ .../notification-channel-cards.test.tsx | 64 ++++++++++--- .../notification-channel-toggle.test.ts | 93 ++++++++++++++++++ .../__tests__/save-model-guard.test.ts | 65 ++++++++++++- src/components/settings/channel-form-dirty.ts | 46 +++------ src/components/settings/email-card.tsx | 31 +++--- .../settings/notification-channel-toggle.ts | 95 +++++++++++++++++++ src/components/settings/ntfy-card.tsx | 27 +++--- src/components/settings/telegram-card.tsx | 58 +++++------ src/components/settings/webhook-card.tsx | 26 ++--- 11 files changed, 413 insertions(+), 189 deletions(-) create mode 100644 src/components/settings/__tests__/notification-channel-toggle.test.ts create 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 b2b0e8061..ea8b68a90 100644 --- a/e2e/notification-dispatch-journey.spec.ts +++ b/e2e/notification-dispatch-journey.spec.ts @@ -270,6 +270,21 @@ 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. @@ -277,10 +292,11 @@ async function saveCard( 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/components/settings/__tests__/channel-form-dirty.test.ts b/src/components/settings/__tests__/channel-form-dirty.test.ts index cb20527dc..63e57592c 100644 --- a/src/components/settings/__tests__/channel-form-dirty.test.ts +++ b/src/components/settings/__tests__/channel-form-dirty.test.ts @@ -21,7 +21,6 @@ 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", () => { @@ -57,12 +56,7 @@ 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: "", - enabled: false, - }; + const form = { serverUrl: "https://ntfy.sh", topic: "t", authToken: "" }; expect(isNtfyFormDirty(form, saved)).toBe(false); expect(isNtfyFormDirty({ ...form, topic: "u" }, saved)).toBe(true); expect(isNtfyFormDirty({ ...form, serverUrl: "https://x" }, saved)).toBe( @@ -73,86 +67,39 @@ describe("channel form dirty state", () => { it("email: the recipient counts", () => { expect( - isEmailFormDirty( - { recipient: "a@x.io", enabled: false }, - { recipient: "a@x.io" }, - ), + isEmailFormDirty({ recipient: "a@x.io" }, { recipient: "a@x.io" }), ).toBe(false); expect( - isEmailFormDirty( - { recipient: "b@x.io", enabled: false }, - { recipient: "a@x.io" }, - ), + isEmailFormDirty({ recipient: "b@x.io" }, { 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", enabled: false }, - { chatId: "1" }, - ), + isTelegramFormDirty({ botToken: "", chatId: "1" }, { chatId: "1" }), + ).toBe(false); + expect( + isTelegramFormDirty({ botToken: " ", chatId: "" }, { chatId: null }), ).toBe(false); expect( isTelegramFormDirty( - { botToken: " ", chatId: "", enabled: false }, - { chatId: null }, - ), - ).toBe(false); - expect( - isTelegramFormDirty( - { botToken: "123:abc", chatId: "1", enabled: false }, + { botToken: "123:abc", chatId: "1" }, { chatId: "1" }, ), ).toBe(true); expect( - isTelegramFormDirty( - { botToken: "", chatId: "2", enabled: false }, - { chatId: "1" }, - ), + isTelegramFormDirty({ botToken: "", chatId: "2" }, { 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", enabled: true }, - undefined, - ), + isNtfyFormDirty({ serverUrl: "", topic: "", authToken: "x" }, undefined), ).toBe(false); - expect(isEmailFormDirty({ recipient: "x", enabled: true }, undefined)).toBe( + expect(isEmailFormDirty({ recipient: "x" }, undefined)).toBe(false); + expect(isTelegramFormDirty({ botToken: "x", chatId: "" }, undefined)).toBe( false, ); - expect( - isTelegramFormDirty( - { botToken: "x", chatId: "", enabled: true }, - undefined, - ), - ).toBe(false); }); }); diff --git a/src/components/settings/__tests__/notification-channel-cards.test.tsx b/src/components/settings/__tests__/notification-channel-cards.test.tsx index 26a2d5b2a..0e09da9cf 100644 --- a/src/components/settings/__tests__/notification-channel-cards.test.tsx +++ b/src/components/settings/__tests__/notification-channel-cards.test.tsx @@ -126,22 +126,56 @@ describe("notification channel cards", () => { ["email", ], ["telegram", ], ] as const)( - "%s enable switch is a form field: it writes nothing on its own", - async (_channel, card) => { + "%s enable switch saves on tap, alone, outside the form", + 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"'); + // The switch precedes the form: its Save covers the fields, not the + // switch. + expect(html.indexOf('role="switch"')).toBeGreaterThan(-1); + expect(html.indexOf('role="switch"')).toBeLessThan(html.indexOf(") { - const body = JSON.parse(init.body) as Record; - expect(Object.keys(body).length).toBeGreaterThan(1); - } + + // The card's first mutation is the switch's, and it sends `enabled` + // and nothing else. + await mocks.mutationOptions[0]!.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([ + [, ["settings", "webhook"]], + [, ["settings", "ntfy"]], + [, ["settings", "email"]], + [, ["telegram", "settings"]], + ] as const)( + "rolls each switch back to its previous server state on error", + async (card, queryKey) => { + const previous = { enabled: false, persistedConfig: "unchanged" }; + mocks.queryClient.getQueryData.mockReturnValue(previous); + render(card); + const toggle = mocks.mutationOptions[0]!; + + 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, + ); }, ); @@ -153,7 +187,7 @@ describe("notification channel cards", () => { expect(html).toContain('