fix(settings): hold the channel test until changes are saved

The webhook, ntfy, email and Telegram test routes send through the
saved channel config. After a field changed, the card kept showing the
Saved note from the last save and Test still ran, so it reported on the
old settings: a webhook switched to the Gotify format was tested with
the generic body and Gotify refused it. The note now disappears on a
change, and the test button stays off with a save-first hint while the
form differs from what is saved. Refs #947
This commit is contained in:
Marc-André Bombeck committed 2026-09-13 18:28:47 +02:00
1 parent d4766a580b
commit 3bb08c3c8a
16 files changed
+268 -8

No files matched your search

+1
View File
@@ -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": {
+1
View File
@@ -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": {
+1
View File
@@ -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": {
+1
View File
@@ -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": {
+1
View File
@@ -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": {
+1
View File
@@ -6133,6 +6133,7 @@
},
"testConnection": {
"test": "연결 테스트",
"saveFirst": "먼저 변경 사항을 저장해 주세요. 테스트는 저장된 설정을 사용해요.",
"testing": "테스트 중…",
"ok": "연결됨 (지연 {latency}ms)",
"errors": {
+1
View File
@@ -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": {
@@ -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,
);
});
});
@@ -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", <WebhookCard key="webhook" isAuthenticated />],
["ntfy", <NtfyCard key="ntfy" isAuthenticated />],
["email", <EmailCard key="email" isAuthenticated />],
["telegram", <TelegramCard key="telegram" isAuthenticated />],
] 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", <WebhookCard key="webhook" isAuthenticated />, 1],
["ntfy", <NtfyCard key="ntfy" isAuthenticated />, 1],
@@ -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(
<I18nProvider initialLocale="en">
<TestConnectionButton
endpoint="/api/settings/webhook/test"
unsavedChanges={unsavedChanges}
/>
</I18nProvider>,
);
}
// 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(/<button[^>]*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(/<button[^>]*disabled=""/);
expect(html).not.toContain("test-connection-save-first");
});
});
@@ -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 ?? "");
}
+4 -1
View File
@@ -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 }) {
/>
</div>
{saveMsg && (
{saveMsg && (saveMsgType === "error" || !dirty) && (
<p
role="alert"
className={`text-sm ${saveMsgType === "success" ? "text-success" : "text-destructive"}`}
@@ -167,6 +169,7 @@ export function EmailCard({ isAuthenticated }: { isAuthenticated: boolean }) {
<TestConnectionButton
endpoint="/api/settings/email/test"
disabled={!settings?.enabled}
unsavedChanges={dirty}
/>
<Button
type="submit"
+4 -1
View File
@@ -13,6 +13,7 @@ import { PasswordInput } from "@/components/ui/password-input";
import { SettingsCard } from "@/components/settings/settings-card";
import { SettingsCardHeader } from "@/components/settings/_card-header";
import { TestConnectionButton } from "@/components/settings/test-connection-button";
import { isNtfyFormDirty } from "@/components/settings/channel-form-dirty";
import {
optimisticallySetChannelEnabled,
persistChannelEnabled,
@@ -58,6 +59,7 @@ export function NtfyCard({ isAuthenticated }: { isAuthenticated: boolean }) {
setServerUrl(settings!.serverUrl);
setTopic(settings!.topic);
}
const dirty = isNtfyFormDirty({ serverUrl, topic, authToken }, settings);
const save = useMutation({
mutationFn: async (enabled: boolean) => {
@@ -196,7 +198,7 @@ export function NtfyCard({ isAuthenticated }: { isAuthenticated: boolean }) {
</div>
</div>
{saveMsg && (
{saveMsg && (saveMsgType === "error" || !dirty) && (
<p
role="alert"
className={`text-sm ${saveMsgType === "success" ? "text-success" : "text-destructive"}`}
@@ -209,6 +211,7 @@ export function NtfyCard({ isAuthenticated }: { isAuthenticated: boolean }) {
<TestConnectionButton
endpoint="/api/settings/ntfy/test"
disabled={!settings?.enabled}
unsavedChanges={dirty}
/>
<Button
type="submit"
+4 -1
View File
@@ -14,6 +14,7 @@ import { PasswordInput } from "@/components/ui/password-input";
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,
@@ -60,6 +61,7 @@ export function TelegramCard({
setSeededKey(settingsKey);
if (settings!.chatId) setChatId(settings!.chatId);
}
const dirty = isTelegramFormDirty({ botToken, chatId }, settings);
const toggleEnabled = useMutation<
void,
@@ -200,7 +202,7 @@ export function TelegramCard({
</ol>
</SettingsInfoTile>
{msg && (
{msg && (msgType === "error" || !dirty) && (
<p
role="alert"
className={`text-sm ${
@@ -215,6 +217,7 @@ export function TelegramCard({
<TestConnectionButton
endpoint="/api/settings/telegram/test"
disabled={!settings?.hasBotToken}
unsavedChanges={dirty}
/>
<Button
type="submit"
@@ -25,6 +25,12 @@ export interface TestConnectionButtonProps {
disabled?: boolean;
/** Label override for the button (defaults to settings.testConnection.test). */
label?: string;
/**
* The card's form differs from what is saved. The test route sends through
* the saved config, so the button waits for a save and says so instead of
* testing something other than what is on screen.
*/
unsavedChanges?: boolean;
}
interface TestResponse {
@@ -128,6 +134,7 @@ export function TestConnectionButton({
endpoint,
disabled = false,
label,
unsavedChanges = false,
}: TestConnectionButtonProps) {
const { t } = useTranslations();
const [testing, setTesting] = useState(false);
@@ -161,7 +168,7 @@ export function TestConnectionButton({
size="sm"
className="min-h-11"
onClick={handleClick}
disabled={disabled || testing}
disabled={disabled || testing || unsavedChanges}
>
{testing ? (
<Loader2 className="h-3.5 w-3.5 animate-spin motion-reduce:animate-none" />
@@ -173,7 +180,16 @@ export function TestConnectionButton({
: (label ?? t("settings.testConnection.test"))}
</Button>
{result?.kind === "ok" && (
{unsavedChanges && (
<p
data-slot="test-connection-save-first"
className="text-muted-foreground text-xs"
>
{t("settings.testConnection.saveFirst")}
</p>
)}
{!unsavedChanges && result?.kind === "ok" && (
<p
role="status"
className="text-success flex items-center gap-1.5 text-xs"
@@ -183,7 +199,7 @@ export function TestConnectionButton({
</p>
)}
{result?.kind === "error" && (
{!unsavedChanges && result?.kind === "error" && (
<TestConnectionFailure
message={describeError(result.errorCode)}
upstreamStatus={result.upstreamStatus}
+7 -1
View File
@@ -14,6 +14,7 @@ import { NativeSelect } from "@/components/ui/native-select";
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,
@@ -63,6 +64,10 @@ export function WebhookCard({ isAuthenticated }: { isAuthenticated: boolean }) {
setHeaderName(settings!.headerName);
setFormat(settings!.format === "gotify" ? "gotify" : "generic");
}
const dirty = isWebhookFormDirty(
{ url, headerName, headerValue, format },
settings,
);
const save = useMutation({
mutationFn: async (enabled: boolean) => {
@@ -238,7 +243,7 @@ export function WebhookCard({ isAuthenticated }: { isAuthenticated: boolean }) {
</div>
</div>
{saveMsg && (
{saveMsg && (saveMsgType === "error" || !dirty) && (
<p
role="alert"
className={`text-sm ${saveMsgType === "success" ? "text-success" : "text-destructive"}`}
@@ -251,6 +256,7 @@ export function WebhookCard({ isAuthenticated }: { isAuthenticated: boolean }) {
<TestConnectionButton
endpoint="/api/settings/webhook/test"
disabled={!settings?.enabled}
unsavedChanges={dirty}
/>
<Button
type="submit"