diff --git a/messages/de.json b/messages/de.json index cbc904085..b6ef44809 100644 --- a/messages/de.json +++ b/messages/de.json @@ -6133,6 +6133,7 @@ }, "testConnection": { "test": "Verbindung testen", + "saveFirst": "Speichere zuerst deine Änderungen. Der Test nutzt die gespeicherten Einstellungen.", "testing": "Teste…", "ok": "Verbunden (Latenz {latency} ms)", "errors": { diff --git a/messages/en.json b/messages/en.json index 2230b740e..ee80a5ff9 100644 --- a/messages/en.json +++ b/messages/en.json @@ -6133,6 +6133,7 @@ }, "testConnection": { "test": "Test connection", + "saveFirst": "Save your changes first. Test uses the saved settings.", "testing": "Testing…", "ok": "Connected (latency {latency} ms)", "errors": { diff --git a/messages/es.json b/messages/es.json index 4d28cc37f..42dd44b5e 100644 --- a/messages/es.json +++ b/messages/es.json @@ -6133,6 +6133,7 @@ }, "testConnection": { "test": "Probar conexión", + "saveFirst": "Guarda primero tus cambios. La prueba usa los ajustes guardados.", "testing": "Probando…", "ok": "Conectado (latencia {latency} ms)", "errors": { diff --git a/messages/fr.json b/messages/fr.json index 58ecbe43e..894ef16da 100644 --- a/messages/fr.json +++ b/messages/fr.json @@ -6133,6 +6133,7 @@ }, "testConnection": { "test": "Tester la connexion", + "saveFirst": "Enregistrez d’abord vos modifications. Le test utilise les réglages enregistrés.", "testing": "Test en cours…", "ok": "Connecté (latence {latency} ms)", "errors": { diff --git a/messages/it.json b/messages/it.json index 9003b0872..f713b1e6a 100644 --- a/messages/it.json +++ b/messages/it.json @@ -6133,6 +6133,7 @@ }, "testConnection": { "test": "Prova connessione", + "saveFirst": "Salva prima le modifiche. La prova usa le impostazioni salvate.", "testing": "Test in corso…", "ok": "Connesso (latenza {latency} ms)", "errors": { diff --git a/messages/ko.json b/messages/ko.json index 6bd7fcd53..d33b4b5b1 100644 --- a/messages/ko.json +++ b/messages/ko.json @@ -6133,6 +6133,7 @@ }, "testConnection": { "test": "연결 테스트", + "saveFirst": "먼저 변경 사항을 저장해 주세요. 테스트는 저장된 설정을 사용해요.", "testing": "테스트 중…", "ok": "연결됨 (지연 {latency}ms)", "errors": { diff --git a/messages/pl.json b/messages/pl.json index 06f0da168..e3b2ab22f 100644 --- a/messages/pl.json +++ b/messages/pl.json @@ -6133,6 +6133,7 @@ }, "testConnection": { "test": "Testuj połączenie", + "saveFirst": "Najpierw zapisz zmiany. Test używa zapisanych ustawień.", "testing": "Testowanie…", "ok": "Połączono (opóźnienie {latency} ms)", "errors": { diff --git a/src/components/settings/__tests__/channel-form-dirty.test.ts b/src/components/settings/__tests__/channel-form-dirty.test.ts new file mode 100644 index 000000000..63e57592c --- /dev/null +++ b/src/components/settings/__tests__/channel-form-dirty.test.ts @@ -0,0 +1,105 @@ +import { describe, expect, it } from "vitest"; + +import { + isEmailFormDirty, + isNtfyFormDirty, + isTelegramFormDirty, + isWebhookFormDirty, +} from "../channel-form-dirty"; + +// The Test button sends through the saved config, so each card has to know +// when its form holds something the server does not (#947: a Gotify format +// picked after the last save was tested as the old generic body). +describe("channel form dirty state", () => { + const webhookSaved = { + url: "https://gotify.example.com/message", + headerName: "X-Gotify-Key", + format: "gotify", + }; + const webhookForm = { + url: "https://gotify.example.com/message", + headerName: "X-Gotify-Key", + headerValue: "", + format: "gotify" as const, + }; + + it("webhook: a form that matches the saved config is clean", () => { + expect(isWebhookFormDirty(webhookForm, webhookSaved)).toBe(false); + }); + + it("webhook: a changed format, URL, header name or a typed header value is a change", () => { + expect( + isWebhookFormDirty(webhookForm, { ...webhookSaved, format: undefined }), + ).toBe(true); + expect( + isWebhookFormDirty( + { ...webhookForm, url: "https://gotify.example.com/message?token=x" }, + webhookSaved, + ), + ).toBe(true); + expect( + isWebhookFormDirty({ ...webhookForm, headerName: "" }, webhookSaved), + ).toBe(true); + expect( + isWebhookFormDirty({ ...webhookForm, headerValue: "tok" }, webhookSaved), + ).toBe(true); + }); + + it("webhook: a config saved before the format choice reads as generic", () => { + expect( + isWebhookFormDirty( + { ...webhookForm, format: "generic" }, + { ...webhookSaved, format: undefined }, + ), + ).toBe(false); + }); + + 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: "" }; + expect(isNtfyFormDirty(form, saved)).toBe(false); + expect(isNtfyFormDirty({ ...form, topic: "u" }, saved)).toBe(true); + expect(isNtfyFormDirty({ ...form, serverUrl: "https://x" }, saved)).toBe( + true, + ); + expect(isNtfyFormDirty({ ...form, authToken: "a" }, saved)).toBe(true); + }); + + it("email: the recipient counts", () => { + expect( + isEmailFormDirty({ recipient: "a@x.io" }, { recipient: "a@x.io" }), + ).toBe(false); + expect( + 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" }, { chatId: "1" }), + ).toBe(false); + expect( + isTelegramFormDirty({ botToken: " ", chatId: "" }, { chatId: null }), + ).toBe(false); + expect( + isTelegramFormDirty( + { botToken: "123:abc", chatId: "1" }, + { chatId: "1" }, + ), + ).toBe(true); + expect( + isTelegramFormDirty({ botToken: "", chatId: "2" }, { chatId: "1" }), + ).toBe(true); + }); + + it("nothing loaded yet is never a change", () => { + expect(isWebhookFormDirty(webhookForm, undefined)).toBe(false); + expect( + isNtfyFormDirty({ serverUrl: "", topic: "", authToken: "x" }, undefined), + ).toBe(false); + expect(isEmailFormDirty({ recipient: "x" }, undefined)).toBe(false); + expect(isTelegramFormDirty({ botToken: "x", chatId: "" }, 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 81499de41..85861ac18 100644 --- a/src/components/settings/__tests__/notification-channel-cards.test.tsx +++ b/src/components/settings/__tests__/notification-channel-cards.test.tsx @@ -9,6 +9,7 @@ const mocks = vi.hoisted(() => ({ onError?: (error: Error, enabled: boolean, previous: unknown) => void; }>, apiFetchRaw: vi.fn(), + testButtonProps: [] as Array<{ endpoint: string; unsavedChanges?: boolean }>, queryClient: { cancelQueries: vi.fn(), getQueryData: vi.fn(), @@ -70,7 +71,13 @@ vi.mock("@/lib/api/api-fetch", () => ({ })); vi.mock("../test-connection-button", () => ({ - TestConnectionButton: () => null, + TestConnectionButton: (props: { + endpoint: string; + unsavedChanges?: boolean; + }) => { + mocks.testButtonProps.push(props); + return null; + }, })); import { I18nProvider } from "@/lib/i18n/context"; @@ -88,6 +95,7 @@ function render(card: ReactElement) { beforeEach(() => { vi.clearAllMocks(); mocks.mutationOptions.length = 0; + mocks.testButtonProps.length = 0; mocks.queryClient.cancelQueries.mockResolvedValue(undefined); mocks.apiFetchRaw.mockResolvedValue( new Response(JSON.stringify({ data: { saved: true }, error: null }), { @@ -97,6 +105,21 @@ beforeEach(() => { }); describe("notification channel cards", () => { + it.each([ + ["webhook", ], + ["ntfy", ], + ["email", ], + ["telegram", ], + ] as const)( + "%s hands its unsaved-change state to the test button, clean once seeded from the server", + (channel, card) => { + render(card); + const props = mocks.testButtonProps.at(-1); + expect(props?.endpoint).toBe(`/api/settings/${channel}/test`); + expect(props?.unsavedChanges).toBe(false); + }, + ); + it.each([ ["webhook", , 1], ["ntfy", , 1], diff --git a/src/components/settings/__tests__/test-connection-button-unsaved.test.tsx b/src/components/settings/__tests__/test-connection-button-unsaved.test.tsx new file mode 100644 index 000000000..27d7d67a8 --- /dev/null +++ b/src/components/settings/__tests__/test-connection-button-unsaved.test.tsx @@ -0,0 +1,37 @@ +import { renderToStaticMarkup } from "react-dom/server"; +import { describe, expect, it, vi } from "vitest"; + +vi.mock("@/lib/api/api-fetch", () => ({ apiFetchRaw: vi.fn() })); + +import { I18nProvider } from "@/lib/i18n/context"; +import { TestConnectionButton } from "../test-connection-button"; + +function render(unsavedChanges?: boolean) { + return renderToStaticMarkup( + + + , + ); +} + +// The test route sends through the saved config. With an unsaved change on +// the card, a test would report on something other than what is on screen. +describe("TestConnectionButton with unsaved changes", () => { + it("locks the button and says to save first", () => { + const html = render(true); + expect(html).toMatch(/]*disabled=""/); + expect(html).toContain('data-slot="test-connection-save-first"'); + expect(html).toContain( + "Save your changes first. Test uses the saved settings.", + ); + }); + + it("stays usable and quiet when the form matches what is saved", () => { + const html = render(false); + expect(html).not.toMatch(/]*disabled=""/); + expect(html).not.toContain("test-connection-save-first"); + }); +}); diff --git a/src/components/settings/channel-form-dirty.ts b/src/components/settings/channel-form-dirty.ts new file mode 100644 index 000000000..24fd4d928 --- /dev/null +++ b/src/components/settings/channel-form-dirty.ts @@ -0,0 +1,57 @@ +/** + * Whether a notification channel card's form differs from what is saved. + * + * The Test button on these cards always sends through the saved channel + * config, never the form. While the form holds an unsaved change, a test + * would report on something other than what is on screen, and a "Saved" + * note left over from an earlier save would say the opposite of the truth. + * A secret field (header value, auth token, bot token) is write-only: the + * server never returns it, so any typed value counts as a change. + */ + +export function isWebhookFormDirty( + form: { + url: string; + headerName: string; + headerValue: string; + format: "generic" | "gotify"; + }, + saved: { url: string; headerName: string; format?: string } | undefined, +): boolean { + if (!saved) return false; + const savedFormat = saved.format === "gotify" ? "gotify" : "generic"; + return ( + form.url !== saved.url || + form.headerName !== saved.headerName || + form.headerValue !== "" || + form.format !== savedFormat + ); +} + +export function isNtfyFormDirty( + form: { serverUrl: string; topic: string; authToken: string }, + saved: { serverUrl: string; topic: string } | undefined, +): boolean { + if (!saved) return false; + return ( + form.serverUrl !== saved.serverUrl || + form.topic !== saved.topic || + form.authToken !== "" + ); +} + +export function isEmailFormDirty( + form: { recipient: string }, + saved: { recipient: string } | undefined, +): boolean { + if (!saved) return false; + return form.recipient !== saved.recipient; +} + +export function isTelegramFormDirty( + form: { botToken: string; chatId: string }, + saved: { chatId?: string | null } | undefined, +): boolean { + if (!saved) return false; + return form.botToken.trim() !== "" || form.chatId !== (saved.chatId ?? ""); +} diff --git a/src/components/settings/email-card.tsx b/src/components/settings/email-card.tsx index 4df8ae511..5cd80fe91 100644 --- a/src/components/settings/email-card.tsx +++ b/src/components/settings/email-card.tsx @@ -12,6 +12,7 @@ import { Switch } from "@/components/ui/switch"; 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, @@ -52,6 +53,7 @@ export function EmailCard({ isAuthenticated }: { isAuthenticated: boolean }) { setSeededKey(settingsKey); setRecipient(settings!.recipient); } + const dirty = isEmailFormDirty({ recipient }, settings); const save = useMutation({ mutationFn: async (enabled: boolean) => { @@ -154,7 +156,7 @@ export function EmailCard({ isAuthenticated }: { isAuthenticated: boolean }) { /> - {saveMsg && ( + {saveMsg && (saveMsgType === "error" || !dirty) && ( { @@ -196,7 +198,7 @@ export function NtfyCard({ isAuthenticated }: { isAuthenticated: boolean }) { - {saveMsg && ( + {saveMsg && (saveMsgType === "error" || !dirty) && ( - {msg && ( + {msg && (msgType === "error" || !dirty) && ( {testing ? ( @@ -173,7 +180,16 @@ export function TestConnectionButton({ : (label ?? t("settings.testConnection.test"))} - {result?.kind === "ok" && ( + {unsavedChanges && ( + + {t("settings.testConnection.saveFirst")} + + )} + + {!unsavedChanges && result?.kind === "ok" && ( )} - {result?.kind === "error" && ( + {!unsavedChanges && result?.kind === "error" && ( { @@ -238,7 +243,7 @@ export function WebhookCard({ isAuthenticated }: { isAuthenticated: boolean }) { - {saveMsg && ( + {saveMsg && (saveMsgType === "error" || !dirty) && (
+ {t("settings.testConnection.saveFirst")} +
)} - {result?.kind === "error" && ( + {!unsavedChanges && result?.kind === "error" && ( { @@ -238,7 +243,7 @@ export function WebhookCard({ isAuthenticated }: { isAuthenticated: boolean }) { - {saveMsg && ( + {saveMsg && (saveMsgType === "error" || !dirty) && (