From 32eaf7a29e41999bd58d3148e4360cbd1c479a3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sun, 13 Sep 2026 14:50:33 +0200 Subject: [PATCH] fix(admin): show each channel's cause in the notification test toast The admin test response carried a code and status per channel that nothing rendered. The toast now lists each failing channel with the translated cause and the HTTP status or SMTP code. The relay reply is no longer part of that response, and the ntfy error no longer echoes the topic, which the sender treats as a secret. Refs #947 --- .../test/__tests__/route.test.ts | 4 +- src/app/api/admin/notifications/test/route.ts | 32 ++++---- .../notification-test-failures.test.ts | 76 +++++++++++++++++++ .../admin/notification-test-failures.ts | 46 +++++++++++ src/components/admin/reminders-section.tsx | 24 ++++-- 5 files changed, 157 insertions(+), 25 deletions(-) create mode 100644 src/components/admin/__tests__/notification-test-failures.test.ts create mode 100644 src/components/admin/notification-test-failures.ts diff --git a/src/app/api/admin/notifications/test/__tests__/route.test.ts b/src/app/api/admin/notifications/test/__tests__/route.test.ts index 259a35461..796464b39 100644 --- a/src/app/api/admin/notifications/test/__tests__/route.test.ts +++ b/src/app/api/admin/notifications/test/__tests__/route.test.ts @@ -347,6 +347,7 @@ describe("POST /api/admin/notifications/test — a failure keeps its cause (#947 upstreamStatus: 400, }); expect(body.data.results[0].error).toContain("HTTP 400"); + expect(body.data.results[0].error).not.toContain("hl-test"); }); it("tests the webhook channel rather than calling it unknown", async () => { @@ -372,8 +373,9 @@ describe("POST /api/admin/notifications/test — a failure keeps its cause (#947 success: false, errorCode: "upstream_rejected", upstreamStatus: 400, - upstreamBody: "priority must be an integer", }); + // The relay body belongs to the settings card, not the admin toast. + expect(body.data.results[0].upstreamBody).toBeUndefined(); }); it("tests the email channel and forwards the SMTP code", async () => { diff --git a/src/app/api/admin/notifications/test/route.ts b/src/app/api/admin/notifications/test/route.ts index 0bf282198..4c71a805e 100644 --- a/src/app/api/admin/notifications/test/route.ts +++ b/src/app/api/admin/notifications/test/route.ts @@ -72,11 +72,12 @@ async function ensureTelegramChannel(userId: string): Promise { return true; } +/** The admin toast renders `errorCode` and the status; the relay body stays on the settings card. */ type TestResult = { channel: string; success: boolean; error?: string; -} & TestFailureDetail; +} & Omit; /** * A failed HTTP or SMTP send, with the code, status and SMTP reply code the @@ -87,21 +88,26 @@ function failedSend( channel: string, label: string, result: SendOutcome, - context?: string, ): TestResult { - const detail: TestFailureDetail = result.errorCode - ? { errorCode: result.errorCode } + const { errorCode, upstreamStatus, smtpCode } = result.errorCode + ? { + errorCode: result.errorCode, + upstreamStatus: undefined, + smtpCode: undefined, + } : testFailureDetail(result); const cause = result.errorCode ? `refused by the private-origin policy (${result.errorCode})` - : detail.errorCode - ? testFailureSentence(label, detail) + : errorCode + ? testFailureSentence(label, { errorCode, upstreamStatus, smtpCode }) : (result.reason ?? "send failed"); return { channel, success: false, - error: context ? `${cause} (${context})` : cause, - ...detail, + error: cause, + ...(errorCode ? { errorCode } : {}), + ...(upstreamStatus !== undefined ? { upstreamStatus } : {}), + ...(smtpCode !== undefined ? { smtpCode } : {}), }; } @@ -205,14 +211,8 @@ export const POST = apiHandler(async () => { const ntfyResult = await sendViaNtfy(config, payload); success = ntfyResult.ok; if (!success) { - results.push( - failedSend( - "NTFY", - "The ntfy server", - ntfyResult, - `topic: ${config.topic}`, - ), - ); + // The topic is not echoed: on a public server it is the secret. + results.push(failedSend("NTFY", "The ntfy server", ntfyResult)); continue; } break; diff --git a/src/components/admin/__tests__/notification-test-failures.test.ts b/src/components/admin/__tests__/notification-test-failures.test.ts new file mode 100644 index 000000000..b2fd9f403 --- /dev/null +++ b/src/components/admin/__tests__/notification-test-failures.test.ts @@ -0,0 +1,76 @@ +/** + * The admin notification test toast names each failing channel's cause + * (review L5): the code the route sends is translated and the status is + * shown, so the per-channel fields in the response have a reader. + */ +import { readFileSync } from "node:fs"; +import { join } from "node:path"; +import { describe, expect, it } from "vitest"; + +import { describeChannelTestFailures } from "../notification-test-failures"; + +const MESSAGES: Record = { + "settings.testConnection.errors.upstream_rejected": + "The other end refused the message", + "settings.testConnection.errors.credentials_rejected": "Credentials rejected", + "admin.notificationTestFailed": "Failed to send test notification", +}; +const t = (key: string) => MESSAGES[key] ?? key; + +describe("describeChannelTestFailures", () => { + it("lists only failing channels, with the translated cause and the status", () => { + expect( + describeChannelTestFailures( + [ + { channel: "APNS", success: true }, + { + channel: "WEBHOOK", + success: false, + error: "The webhook answered HTTP 400.", + errorCode: "upstream_rejected", + upstreamStatus: 400, + }, + { + channel: "EMAIL", + success: false, + error: "The mail server rejected the credentials (SMTP 535).", + errorCode: "credentials_rejected", + smtpCode: 535, + }, + ], + t, + ), + ).toEqual([ + "Webhook: The other end refused the message (HTTP 400)", + "Email: Credentials rejected (SMTP 535)", + ]); + }); + + it("falls back to the route's sentence when there is no known code", () => { + expect( + describeChannelTestFailures( + [ + { + channel: "NTFY", + success: false, + error: "SYSTEM_ALERT disabled in settings", + }, + { channel: "MYSTERY", success: false, errorCode: "unlisted" }, + ], + t, + ), + ).toEqual([ + "ntfy: SYSTEM_ALERT disabled in settings", + "MYSTERY: Failed to send test notification", + ]); + }); + + it("is what the admin toast renders", () => { + const src = readFileSync( + join(process.cwd(), "src/components/admin/reminders-section.tsx"), + "utf8", + ); + expect(src).toMatch(/describeChannelTestFailures\(data\?\.results/); + expect(src).toMatch(/description:/); + }); +}); diff --git a/src/components/admin/notification-test-failures.ts b/src/components/admin/notification-test-failures.ts new file mode 100644 index 000000000..caf578b50 --- /dev/null +++ b/src/components/admin/notification-test-failures.ts @@ -0,0 +1,46 @@ +import { + CHANNEL_TYPE_LABELS, + type ChannelType, +} from "@/lib/notifications/types"; + +/** One channel's row in the admin notification test response. */ +export interface ChannelTestResult { + channel: string; + success: boolean; + error?: string; + errorCode?: string; + upstreamStatus?: number; + smtpCode?: number; +} + +/** + * One line per failing channel for the admin test toast: the channel name, + * the cause in the admin's language (the same `settings.testConnection.errors` + * vocabulary the settings cards use), and the HTTP status or SMTP reply code. + * A row without a known code falls back to the route's own sentence. + */ +export function describeChannelTestFailures( + results: ReadonlyArray, + t: (key: string) => string, +): string[] { + return results + .filter((r) => !r.success) + .map((r) => { + const label = CHANNEL_TYPE_LABELS[r.channel as ChannelType] ?? r.channel; + const key = r.errorCode + ? `settings.testConnection.errors.${r.errorCode}` + : null; + const translated = key ? t(key) : null; + const cause = + translated && translated !== key + ? translated + : (r.error ?? t("admin.notificationTestFailed")); + const code = + typeof r.upstreamStatus === "number" + ? ` (HTTP ${r.upstreamStatus})` + : typeof r.smtpCode === "number" + ? ` (SMTP ${r.smtpCode})` + : ""; + return `${label}: ${cause}${code}`; + }); +} diff --git a/src/components/admin/reminders-section.tsx b/src/components/admin/reminders-section.tsx index ff4f28818..2a6d892d3 100644 --- a/src/components/admin/reminders-section.tsx +++ b/src/components/admin/reminders-section.tsx @@ -22,6 +22,10 @@ import { useTranslations } from "@/lib/i18n/context"; import { queryKeys } from "@/lib/query-keys"; import { useAdminSettings, useUpdateSettings } from "./_shared"; import { apiGet, apiPost } from "@/lib/api/api-fetch"; +import { + describeChannelTestFailures, + type ChannelTestResult, +} from "./notification-test-failures"; interface NotificationHealth { windowHours: number; @@ -132,19 +136,23 @@ export function RemindersSection() { return apiPost< | { message?: string; - results?: Array<{ - channel: string; - success: boolean; - error?: string; - }>; + results?: ChannelTestResult[]; } | undefined >("/api/admin/notifications/test"); }, onSuccess: (data) => { - const hasFailures = data?.results?.some((r) => !r.success); - if (hasFailures) { - toast.error(data?.message ?? t("admin.notificationTestFailed")); + const failures = describeChannelTestFailures(data?.results ?? [], t); + if (failures.length > 0) { + toast.error(data?.message ?? t("admin.notificationTestFailed"), { + description: ( +
    + {failures.map((line) => ( +
  • {line}
    • + ))} +
+ ), + }); } else { toast.success(data?.message ?? t("admin.notificationTestSuccess")); }