From d47e9755747cd6ef897ae9bb33e8b1727caa216f Mon Sep 17 00:00:00 2001
From: Leon <mail@leonmuscat.de>
Date: Wed, 16 Jul 2025 18:20:45 +0200
Subject: [PATCH] feat: toast for settings save

---
 .../components/letterfeed/SettingsDialog.tsx  | 105 ++++++++++++++----
 .../__tests__/SettingsDialog.test.tsx         |  59 +++++++++-
 2 files changed, 137 insertions(+), 27 deletions(-)

diff --git a/frontend/components/letterfeed/SettingsDialog.tsx b/frontend/components/letterfeed/SettingsDialog.tsx
index 324343c..0fc3438 100644
--- a/frontend/components/letterfeed/SettingsDialog.tsx
+++ b/frontend/components/letterfeed/SettingsDialog.tsx
@@ -11,9 +11,21 @@ import {
 import { Input } from "@/components/ui/input"
 import { Label } from "@/components/ui/label"
 import { Checkbox } from "@/components/ui/checkbox"
-import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "@/components/ui/select"
+import {
+  Select,
+  SelectContent,
+  SelectItem,
+  SelectTrigger,
+  SelectValue,
+} from "@/components/ui/select"
 import { Loader2, CheckCircle, XCircle } from "lucide-react"
-import { Settings as AppSettings, SettingsCreate, updateSettings, testImapConnection } from "@/lib/api"
+import {
+  Settings as AppSettings,
+  SettingsCreate,
+  updateSettings,
+  testImapConnection,
+} from "@/lib/api"
+import { toast } from "sonner"
 
 interface SettingsDialogProps {
   settings: AppSettings
@@ -23,9 +35,19 @@ interface SettingsDialogProps {
   onSuccess: () => void
 }
 
-export function SettingsDialog({ settings, folderOptions, isOpen, onOpenChange, onSuccess }: SettingsDialogProps) {
-  const [currentSettings, setCurrentSettings] = useState<SettingsCreate | null>(null)
-  const [testConnectionStatus, setTestConnectionStatus] = useState<"idle" | "loading" | "success" | "error">("idle")
+export function SettingsDialog({
+  settings,
+  folderOptions,
+  isOpen,
+  onOpenChange,
+  onSuccess,
+}: SettingsDialogProps) {
+  const [currentSettings, setCurrentSettings] = useState<SettingsCreate | null>(
+    null
+  )
+  const [testConnectionStatus, setTestConnectionStatus] = useState<
+    "idle" | "loading" | "success" | "error"
+  >("idle")
   const [testConnectionMessage, setTestConnectionMessage] = useState("")
 
   useEffect(() => {
@@ -36,7 +58,10 @@ export function SettingsDialog({ settings, folderOptions, isOpen, onOpenChange,
 
   if (!currentSettings) return null
 
-  const handleSettingsChange = <K extends keyof SettingsCreate>(key: K, value: SettingsCreate[K]) => {
+  const handleSettingsChange = <K extends keyof SettingsCreate>(
+    key: K,
+    value: SettingsCreate[K]
+  ) => {
     setCurrentSettings((prev) => (prev ? { ...prev, [key]: value } : null))
   }
 
@@ -44,10 +69,12 @@ export function SettingsDialog({ settings, folderOptions, isOpen, onOpenChange,
     if (!currentSettings) return
     try {
       await updateSettings(currentSettings)
+      toast.success("Settings saved successfully!")
       onOpenChange(false)
       onSuccess()
     } catch (error) {
       console.error("Failed to save settings:", error)
+      toast.error("Failed to save settings.")
     }
   }
 
@@ -74,18 +101,24 @@ export function SettingsDialog({ settings, folderOptions, isOpen, onOpenChange,
       <DialogContent className="sm:max-w-2xl max-h-[90vh] overflow-y-auto">
         <DialogHeader>
           <DialogTitle>Settings</DialogTitle>
-          <DialogDescription>Fields are locked if they are set by environment variables.</DialogDescription>
+          <DialogDescription>
+            Fields are locked if they are set by environment variables.
+          </DialogDescription>
         </DialogHeader>
         <div className="grid grid-cols-1 md:grid-cols-2 gap-x-8 gap-y-6 py-4">
           {/* IMAP Configuration */}
           <div className="space-y-4">
-            <h3 className="text-lg font-medium col-span-1 md:col-span-2">IMAP Configuration</h3>
+            <h3 className="text-lg font-medium col-span-1 md:col-span-2">
+              IMAP Configuration
+            </h3>
             <div className="space-y-2">
               <Label htmlFor="imap-server">IMAP Server</Label>
               <Input
                 id="imap-server"
                 value={currentSettings.imap_server}
-                onChange={(e) => handleSettingsChange("imap_server", e.target.value)}
+                onChange={(e) =>
+                  handleSettingsChange("imap_server", e.target.value)
+                }
                 placeholder="imap.gmail.com"
                 disabled={settings.locked_fields.includes("imap_server")}
               />
@@ -95,7 +128,9 @@ export function SettingsDialog({ settings, folderOptions, isOpen, onOpenChange,
               <Input
                 id="imap-username"
                 value={currentSettings.imap_username}
-                onChange={(e) => handleSettingsChange("imap_username", e.target.value)}
+                onChange={(e) =>
+                  handleSettingsChange("imap_username", e.target.value)
+                }
                 placeholder="your-email@gmail.com"
                 disabled={settings.locked_fields.includes("imap_username")}
               />
@@ -106,7 +141,9 @@ export function SettingsDialog({ settings, folderOptions, isOpen, onOpenChange,
                 id="imap-password"
                 type="password"
                 value={currentSettings.imap_password}
-                onChange={(e) => handleSettingsChange("imap_password", e.target.value)}
+                onChange={(e) =>
+                  handleSettingsChange("imap_password", e.target.value)
+                }
                 placeholder="Your password or app password"
                 disabled={settings.locked_fields.includes("imap_password")}
               />
@@ -118,18 +155,26 @@ export function SettingsDialog({ settings, folderOptions, isOpen, onOpenChange,
                 variant="outline"
                 size="sm"
               >
-                {testConnectionStatus === "loading" && <Loader2 className="w-4 h-4 mr-2 animate-spin" />}
+                {testConnectionStatus === "loading" && (
+                  <Loader2 className="w-4 h-4 mr-2 animate-spin" />
+                )}
                 Test Connection
               </Button>
               {testConnectionStatus !== "idle" && (
                 <div
                   data-testid="connection-status"
                   className={`mt-2 flex items-center text-sm ${
-                    testConnectionStatus === "success" ? "text-green-600" : "text-red-600"
+                    testConnectionStatus === "success"
+                      ? "text-green-600"
+                      : "text-red-600"
                   }`}
                 >
-                  {testConnectionStatus === "success" && <CheckCircle className="w-4 h-4 mr-2" />}
-                  {testConnectionStatus === "error" && <XCircle className="w-4 h-4 mr-2" />}
+                  {testConnectionStatus === "success" && (
+                    <CheckCircle className="w-4 h-4 mr-2" />
+                  )}
+                  {testConnectionStatus === "error" && (
+                    <XCircle className="w-4 h-4 mr-2" />
+                  )}
                   {testConnectionMessage}
                 </div>
               )}
@@ -138,12 +183,16 @@ export function SettingsDialog({ settings, folderOptions, isOpen, onOpenChange,
 
           {/* Email Processing */}
           <div className="space-y-4">
-            <h3 className="text-lg font-medium col-span-1 md:col-span-2">Email Processing</h3>
+            <h3 className="text-lg font-medium col-span-1 md:col-span-2">
+              Email Processing
+            </h3>
             <div className="space-y-2">
               <Label htmlFor="search-folder">Folder to Search</Label>
               <Select
                 value={currentSettings.search_folder}
-                onValueChange={(value) => handleSettingsChange("search_folder", value)}
+                onValueChange={(value) =>
+                  handleSettingsChange("search_folder", value)
+                }
                 disabled={settings.locked_fields.includes("search_folder")}
               >
                 <SelectTrigger>
@@ -162,7 +211,12 @@ export function SettingsDialog({ settings, folderOptions, isOpen, onOpenChange,
               <Label htmlFor="move-folder">Move to Folder</Label>
               <Select
                 value={currentSettings.move_to_folder || "None"}
-                onValueChange={(value) => handleSettingsChange("move_to_folder", value === "None" ? null : value)}
+                onValueChange={(value) =>
+                  handleSettingsChange(
+                    "move_to_folder",
+                    value === "None" ? null : value
+                  )
+                }
                 disabled={settings.locked_fields.includes("move_to_folder")}
               >
                 <SelectTrigger>
@@ -186,7 +240,12 @@ export function SettingsDialog({ settings, folderOptions, isOpen, onOpenChange,
                 min="1"
                 max="1440"
                 value={currentSettings.email_check_interval}
-                onChange={(e) => handleSettingsChange("email_check_interval", Number.parseInt(e.target.value) || 15)}
+                onChange={(e) =>
+                  handleSettingsChange(
+                    "email_check_interval",
+                    Number.parseInt(e.target.value) || 15
+                  )
+                }
                 placeholder="15"
                 disabled={settings.locked_fields.includes("email_check_interval")}
               />
@@ -195,7 +254,9 @@ export function SettingsDialog({ settings, folderOptions, isOpen, onOpenChange,
               <Checkbox
                 id="mark-read"
                 checked={currentSettings.mark_as_read}
-                onCheckedChange={(checked) => handleSettingsChange("mark_as_read", !!checked)}
+                onCheckedChange={(checked) =>
+                  handleSettingsChange("mark_as_read", !!checked)
+                }
                 disabled={settings.locked_fields.includes("mark_as_read")}
               />
               <Label htmlFor="mark-read" className="text-sm font-normal">
@@ -206,7 +267,9 @@ export function SettingsDialog({ settings, folderOptions, isOpen, onOpenChange,
               <Checkbox
                 id="auto-add"
                 checked={currentSettings.auto_add_new_senders}
-                onCheckedChange={(checked) => handleSettingsChange("auto_add_new_senders", !!checked)}
+                onCheckedChange={(checked) =>
+                  handleSettingsChange("auto_add_new_senders", !!checked)
+                }
                 disabled={settings.locked_fields.includes("auto_add_new_senders")}
               />
               <Label htmlFor="auto-add" className="text-sm font-normal">
diff --git a/frontend/components/letterfeed/__tests__/SettingsDialog.test.tsx b/frontend/components/letterfeed/__tests__/SettingsDialog.test.tsx
index 23b7a48..8a46216 100644
--- a/frontend/components/letterfeed/__tests__/SettingsDialog.test.tsx
+++ b/frontend/components/letterfeed/__tests__/SettingsDialog.test.tsx
@@ -4,6 +4,7 @@ import "@testing-library/jest-dom"
 import { SettingsDialog } from "../SettingsDialog"
 import { Settings } from "@/lib/api"
 import * as api from "@/lib/api"
+import { toast } from "sonner"
 
 // Mock the API module
 jest.mock("@/lib/api", () => ({
@@ -12,6 +13,14 @@ jest.mock("@/lib/api", () => ({
   testImapConnection: jest.fn(),
 }))
 
+// Mock the toast module
+jest.mock("sonner", () => ({
+  toast: {
+    success: jest.fn(),
+    error: jest.fn(),
+  },
+}))
+
 const mockedApi = api as jest.Mocked<typeof api>
 
 const mockSettings: Settings = {
@@ -31,9 +40,15 @@ const mockFolderOptions = ["INBOX", "Sent", "Archive", "Spam"]
 describe("SettingsDialog", () => {
   const handleOpenChange = jest.fn()
   const handleSuccess = jest.fn()
+  const consoleError = jest.spyOn(console, "error").mockImplementation(() => {})
 
   beforeEach(() => {
     jest.clearAllMocks()
+    consoleError.mockClear()
+  })
+
+  afterAll(() => {
+    consoleError.mockRestore()
   })
 
   it("renders settings and respects locked fields", () => {
@@ -53,11 +68,13 @@ describe("SettingsDialog", () => {
     expect(screen.getByLabelText(/IMAP Username/i)).toBeEnabled()
 
     // Check that values are set correctly
-    expect(screen.getByLabelText(/IMAP Username/i)).toHaveValue(mockSettings.imap_username)
+    expect(screen.getByLabelText(/IMAP Username/i)).toHaveValue(
+      mockSettings.imap_username
+    )
     expect(screen.getByLabelText(/Mark emails as read/i)).toBeChecked()
   })
 
-  it("allows updating and saving settings", async () => {
+  it("allows updating and saving settings, showing success toast", async () => {
     mockedApi.updateSettings.mockResolvedValueOnce(mockSettings)
 
     render(
@@ -71,7 +88,9 @@ describe("SettingsDialog", () => {
     )
 
     // Change a setting
-    fireEvent.change(screen.getByLabelText(/IMAP Username/i), { target: { value: "new.user@example.com" } })
+    fireEvent.change(screen.getByLabelText(/IMAP Username/i), {
+      target: { value: "new.user@example.com" },
+    })
 
     // Save
     fireEvent.click(screen.getByRole("button", { name: /Save Settings/i }))
@@ -82,13 +101,37 @@ describe("SettingsDialog", () => {
           imap_username: "new.user@example.com",
         })
       )
+      expect(toast.success).toHaveBeenCalledWith("Settings saved successfully!")
       expect(handleSuccess).toHaveBeenCalledTimes(1)
       expect(handleOpenChange).toHaveBeenCalledWith(false)
     })
   })
 
+  it("shows an error toast if saving settings fails", async () => {
+    mockedApi.updateSettings.mockRejectedValueOnce(new Error("Failed to save"))
+
+    render(
+      <SettingsDialog
+        settings={mockSettings}
+        folderOptions={mockFolderOptions}
+        isOpen={true}
+        onOpenChange={handleOpenChange}
+        onSuccess={handleSuccess}
+      />
+    )
+
+    fireEvent.click(screen.getByRole("button", { name: /Save Settings/i }))
+
+    await waitFor(() => {
+      expect(toast.error).toHaveBeenCalledWith("Failed to save settings.")
+      expect(handleSuccess).not.toHaveBeenCalled()
+    })
+  })
+
   it("handles successful connection test", async () => {
-    mockedApi.testImapConnection.mockResolvedValueOnce({ message: "Connection successful!" })
+    mockedApi.testImapConnection.mockResolvedValueOnce({
+      message: "Connection successful!",
+    })
 
     render(
       <SettingsDialog
@@ -104,12 +147,16 @@ describe("SettingsDialog", () => {
 
     await waitFor(() => {
       expect(screen.getByText("Connection successful!")).toBeInTheDocument()
-      expect(screen.getByTestId("connection-status")).toHaveClass("text-green-600")
+      expect(screen.getByTestId("connection-status")).toHaveClass(
+        "text-green-600"
+      )
     })
   })
 
   it("handles failed connection test", async () => {
-    mockedApi.testImapConnection.mockRejectedValueOnce(new Error("Authentication failed"))
+    mockedApi.testImapConnection.mockRejectedValueOnce(
+      new Error("Authentication failed")
+    )
 
     render(
       <SettingsDialog