rework email settings UX with guided flow and friendly errors

grouped providers by category, added per-provider key validation
with cross-provider detection, friendly delivery error messages,
retryable vs config error distinction, from-address in general
settings, and "Save settings" button to match admin conventions

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

test/berrypod/key_validation_test.exs

@@ -170,19 +170,99 @@ defmodule Berrypod.KeyValidationTest do
     end
   end
 
-  describe "validate_email_key/3 - unknown providers" do
-    test "accepts reasonable key for unknown adapter" do
-      assert {:ok, "some_api_key_12345"} =
-               KeyValidation.validate_email_key("some_api_key_12345", "mailersend", "api_key")
+  describe "validate_email_key/3 - Mailjet" do
+    test "accepts valid api_key" do
+      key = String.duplicate("a1b2c3d4", 4)
+      assert {:ok, ^key} = KeyValidation.validate_email_key(key, "mailjet", "api_key")
     end
 
-    test "accepts reasonable key for non-api_key fields" do
+    test "accepts valid secret" do
+      key = String.duplicate("e5f6a7b8", 4)
+      assert {:ok, ^key} = KeyValidation.validate_email_key(key, "mailjet", "secret")
+    end
+
+    test "rejects short api_key" do
+      assert {:error, msg} = KeyValidation.validate_email_key("short", "mailjet", "api_key")
+      assert msg =~ "too short"
+      assert msg =~ "Mailjet"
+    end
+
+    test "rejects short secret" do
+      assert {:error, msg} = KeyValidation.validate_email_key("short", "mailjet", "secret")
+      assert msg =~ "too short"
+      assert msg =~ "Mailjet"
+    end
+  end
+
+  describe "validate_email_key/3 - MailerSend" do
+    test "accepts key with mlsn. prefix" do
+      key = "mlsn." <> String.duplicate("ab", 20)
+      assert {:ok, ^key} = KeyValidation.validate_email_key(key, "mailersend", "api_key")
+    end
+
+    test "accepts long key without known prefix" do
+      key = String.duplicate("x", 40)
+      assert {:ok, ^key} = KeyValidation.validate_email_key(key, "mailersend", "api_key")
+    end
+
+    test "rejects short key" do
+      assert {:error, msg} = KeyValidation.validate_email_key("short", "mailersend", "api_key")
+      assert msg =~ "mlsn."
+    end
+  end
+
+  describe "validate_email_key/3 - MailPace" do
+    test "accepts valid token" do
+      key = "abc123def456ghi789"
+      assert {:ok, ^key} = KeyValidation.validate_email_key(key, "mailpace", "api_key")
+    end
+
+    test "rejects short token" do
+      assert {:error, msg} = KeyValidation.validate_email_key("short", "mailpace", "api_key")
+      assert msg =~ "too short"
+      assert msg =~ "MailPace"
+    end
+  end
+
+  describe "validate_email_key/3 - cross-provider detection" do
+    test "detects Brevo key pasted into SendGrid" do
+      key = "xkeysib-abc123def456"
+      assert {:error, msg} = KeyValidation.validate_email_key(key, "sendgrid", "api_key")
+      assert msg =~ "Brevo key"
+    end
+
+    test "detects SendGrid key pasted into Brevo" do
+      key = "SG.abc123.def456"
+      assert {:error, msg} = KeyValidation.validate_email_key(key, "brevo", "api_key")
+      assert msg =~ "SendGrid key"
+    end
+
+    test "detects Resend key pasted into Postmark" do
+      key = "re_abc123xyz456"
+      assert {:error, msg} = KeyValidation.validate_email_key(key, "postmark", "api_key")
+      assert msg =~ "Resend key"
+    end
+
+    test "detects MailerSend key pasted into Mailgun" do
+      key = "mlsn.abc123"
+      assert {:error, msg} = KeyValidation.validate_email_key(key, "mailgun", "api_key")
+      assert msg =~ "MailerSend key"
+    end
+  end
+
+  describe "validate_email_key/3 - non-api_key fields" do
+    test "accepts reasonable value for domain" do
+      assert {:ok, "mg.example.com"} =
+               KeyValidation.validate_email_key("mg.example.com", "mailgun", "domain")
+    end
+
+    test "accepts reasonable value for relay" do
       assert {:ok, "smtp.example.com"} =
                KeyValidation.validate_email_key("smtp.example.com", "smtp", "relay")
     end
 
     test "rejects very short value" do
-      assert {:error, msg} = KeyValidation.validate_email_key("ab", "mailersend", "api_key")
+      assert {:error, msg} = KeyValidation.validate_email_key("ab", "smtp", "relay")
       assert msg =~ "too short"
     end
 

test/berrypod/mailer_test.exs

@@ -88,6 +88,125 @@ defmodule Berrypod.MailerTest do
     end
   end
 
+  describe "friendly_error/1" do
+    # API errors with status codes
+    test "401 with message body" do
+      error = {401, %{"code" => "unauthorized", "message" => "Key not found"}}
+      result = Mailer.friendly_error(error)
+      assert result =~ "API key was rejected"
+      assert result =~ "key not found"
+    end
+
+    test "401 without message" do
+      assert Mailer.friendly_error({401, ""}) =~ "API key was rejected"
+    end
+
+    test "403 with message" do
+      error = {403, %{"message" => "Insufficient permissions"}}
+      result = Mailer.friendly_error(error)
+      assert result =~ "permission"
+      assert result =~ "insufficient permissions"
+    end
+
+    test "422 with errors array" do
+      error = {422, %{"errors" => [%{"message" => "Invalid sender address"}]}}
+      result = Mailer.friendly_error(error)
+      assert result =~ "rejected"
+      assert result =~ "invalid sender address"
+    end
+
+    test "429 rate limit" do
+      assert Mailer.friendly_error({429, %{}}) =~ "Rate limit"
+    end
+
+    test "500 server error" do
+      assert Mailer.friendly_error({500, %{"message" => "Internal error"}}) =~ "having issues"
+    end
+
+    test "Postmark ErrorMessage format" do
+      error = {422, %{"ErrorCode" => 300, "ErrorMessage" => "Invalid 'From' address"}}
+      result = Mailer.friendly_error(error)
+      assert result =~ "invalid 'From' address"
+    end
+
+    # SMTP errors
+    test "permanent SMTP failure" do
+      assert Mailer.friendly_error({:permanent_failure, "550 User not found"}) =~ "rejected"
+
+      assert Mailer.friendly_error({:permanent_failure, "550 User not found"}) =~
+               "550 User not found"
+    end
+
+    test "temporary SMTP failure" do
+      assert Mailer.friendly_error({:temporary_failure, "421 Try again"}) =~
+               "temporarily unavailable"
+    end
+
+    test "SMTP retries exceeded" do
+      assert Mailer.friendly_error({:retries_exceeded, nil}) =~ "connection failed"
+    end
+
+    test "SMTP auth failed" do
+      assert Mailer.friendly_error({:auth_failed, nil}) =~ "login failed"
+    end
+
+    # Network errors
+    test "timeout" do
+      assert Mailer.friendly_error(:timeout) =~ "timed out"
+    end
+
+    test "connection refused" do
+      assert Mailer.friendly_error(:econnrefused) =~ "Connection refused"
+    end
+
+    test "nxdomain" do
+      assert Mailer.friendly_error(:nxdomain) =~ "hostname not found"
+    end
+
+    # Catch-all
+    test "unknown error falls back to inspect" do
+      assert Mailer.friendly_error({:something, :weird}) =~ "Delivery failed"
+    end
+
+    test "string errors pass through" do
+      assert Mailer.friendly_error("some error") == "some error"
+    end
+  end
+
+  describe "retryable_error?/1" do
+    test "401 is not retryable" do
+      refute Mailer.retryable_error?({401, %{}})
+    end
+
+    test "403 is not retryable" do
+      refute Mailer.retryable_error?({403, %{}})
+    end
+
+    test "429 is retryable" do
+      assert Mailer.retryable_error?({429, %{}})
+    end
+
+    test "500 is retryable" do
+      assert Mailer.retryable_error?({500, %{}})
+    end
+
+    test "timeout is retryable" do
+      assert Mailer.retryable_error?(:timeout)
+    end
+
+    test "temporary SMTP failure is retryable" do
+      assert Mailer.retryable_error?({:temporary_failure, "421 Try again"})
+    end
+
+    test "permanent SMTP failure is not retryable" do
+      refute Mailer.retryable_error?({:permanent_failure, "550 Bad mailbox"})
+    end
+
+    test "auth failure is not retryable" do
+      refute Mailer.retryable_error?({:auth_failed, nil})
+    end
+  end
+
   describe "current_config/0" do
     test "returns {nil, %{}} when no adapter configured" do
       Application.put_env(:berrypod, Mailer, adapter: Swoosh.Adapters.Local)

test/berrypod_web/live/admin/email_settings_test.exs

@@ -29,21 +29,20 @@ defmodule BerrypodWeb.Admin.EmailSettingsTest do
       {:ok, _view, html} = live(conn, ~p"/admin/settings/email")
 
       assert html =~ "Email settings"
-      assert html =~ "Email provider"
+      assert html =~ "Choose a provider"
       # Provider names rendered as radio cards
-      assert html =~ "Postmark"
       assert html =~ "Brevo"
       assert html =~ "Mailjet"
       assert html =~ "MailPace"
       assert html =~ "Postal"
     end
 
-    test "shows provider descriptions", %{conn: conn} do
+    test "shows setup guidance", %{conn: conn} do
       {:ok, _view, html} = live(conn, ~p"/admin/settings/email")
 
-      assert html =~ "Excellent deliverability tracking"
-      assert html =~ "All-in-one platform, GDPR-friendly"
-      assert html =~ "EU data processing"
+      assert html =~ "needs an email provider"
+      assert html =~ "Paste your API key"
+      assert html =~ "300 emails/day free"
     end
 
     test "selecting a provider shows its config fields", %{conn: conn} do
@@ -64,72 +63,60 @@ defmodule BerrypodWeb.Admin.EmailSettingsTest do
     test "selecting a different provider shows different fields", %{conn: conn} do
       {:ok, view, _html} = live(conn, ~p"/admin/settings/email")
 
-      # Select Mailgun which needs api_key + domain
+      # Select Brevo which needs just an api_key
       html =
         view
-        |> form("form[phx-change=\"change_adapter\"]", %{email: %{adapter: "mailgun"}})
+        |> form("form[phx-change=\"change_adapter\"]", %{email: %{adapter: "brevo"}})
         |> render_change()
 
       assert html =~ "API key"
-      assert html =~ "Domain"
+      assert html =~ "Brevo"
     end
 
     test "saving config persists settings", %{conn: conn} do
       {:ok, view, _html} = live(conn, ~p"/admin/settings/email")
 
-      # Select Postmark via form change
+      # Select Brevo via form change
       view
-      |> form("form[phx-change=\"change_adapter\"]", %{email: %{adapter: "postmark"}})
+      |> form("form[phx-change=\"change_adapter\"]", %{email: %{adapter: "brevo"}})
       |> render_change()
 
-      # Submit with an API key (Postmark uses UUID format)
+      # Submit with an API key
       html =
         view
         |> form("form[phx-submit=\"save\"]", %{
-          email: %{adapter: "postmark", api_key: "abc12345-abcd-1234-abcd-123456789abc"}
+          email: %{adapter: "brevo", api_key: "xkeysib-abc123def456"}
         })
         |> render_submit()
 
-      assert html =~ "Email settings saved"
-      assert Settings.get_setting("email_adapter") == "postmark"
+      assert html =~ "Settings saved"
+      assert Settings.get_setting("email_adapter") == "brevo"
     end
 
     test "saving without required fields shows error", %{conn: conn} do
       {:ok, view, _html} = live(conn, ~p"/admin/settings/email")
 
-      # Select Postmark
+      # Select Brevo
       view
-      |> form("form[phx-change=\"change_adapter\"]", %{email: %{adapter: "postmark"}})
+      |> form("form[phx-change=\"change_adapter\"]", %{email: %{adapter: "brevo"}})
       |> render_change()
 
       # Submit without API key
       html =
         view
-        |> form("form[phx-submit=\"save\"]", %{email: %{adapter: "postmark", api_key: ""}})
+        |> form("form[phx-submit=\"save\"]", %{email: %{adapter: "brevo", api_key: ""}})
         |> render_submit()
 
       assert html =~ "API key is required"
     end
 
-    test "disconnect clears email configuration", %{conn: conn} do
-      Settings.put_setting("email_adapter", "postmark")
-      Settings.put_secret("email_postmark_api_key", "pm_test_abc")
-
-      {:ok, view, _html} = live(conn, ~p"/admin/settings/email")
-
-      html = render_click(view, "disconnect")
-
-      assert html =~ "Email provider disconnected"
-      assert is_nil(Settings.get_setting("email_adapter"))
-    end
-
     test "shows test email section when configured", %{conn: conn} do
       Settings.put_setting("email_adapter", "postmark")
       Settings.put_secret("email_postmark_api_key", "pm_test_abc")
 
       {:ok, _view, html} = live(conn, ~p"/admin/settings/email")
 
-      assert html =~ "Test email"
+      assert html =~ "Send a test email"
       assert html =~ "Send test email"
     end
 
@@ -143,7 +130,7 @@ defmodule BerrypodWeb.Admin.EmailSettingsTest do
       refute html =~ "Send test email"
     end
 
-    test "sending test email sets verified flag", %{conn: conn} do
+    test "sending test email shows success and sets verified flag", %{conn: conn} do
       Settings.put_setting("email_adapter", "postmark")
       Settings.put_secret("email_postmark_api_key", "pm_test_abc")
 
@@ -151,10 +138,38 @@ defmodule BerrypodWeb.Admin.EmailSettingsTest do
 
       html = render_click(view, "send_test")
 
-      assert html =~ "Test email sent"
+      assert html =~ "Email is working"
+      assert html =~ "Send again"
       assert Mailer.email_verified?()
     end
 
+    test "switching provider clears old provider settings", %{conn: conn} do
+      # Save Mailjet config first
+      Settings.put_setting("email_adapter", "mailjet")
+      Settings.put_secret("email_mailjet_api_key", "mj-key-123")
+      Settings.put_secret("email_mailjet_secret", "mj-secret-456")
+
+      {:ok, view, _html} = live(conn, ~p"/admin/settings/email")
+
+      # Switch to Brevo and save
+      view
+      |> form("form[phx-change=\"change_adapter\"]", %{email: %{adapter: "brevo"}})
+      |> render_change()
+
+      view
+      |> form("form[phx-submit=\"save\"]", %{
+        email: %{adapter: "brevo", api_key: "xkeysib-switch-test"}
+      })
+      |> render_submit()
+
+      # Old Mailjet settings should be deleted
+      assert Settings.get_setting("email_adapter") == "brevo"
+      refute Settings.has_secret?("email_mailjet_api_key")
+      refute Settings.has_secret?("email_mailjet_secret")
+      # New Brevo key should exist
+      assert Settings.has_secret?("email_brevo_api_key")
+    end
+
     test "saving config clears verified flag", %{conn: conn} do
       Mailer.mark_email_verified()
       assert Mailer.email_verified?()
@@ -162,30 +177,17 @@ defmodule BerrypodWeb.Admin.EmailSettingsTest do
       {:ok, view, _html} = live(conn, ~p"/admin/settings/email")
 
       view
-      |> form("form[phx-change=\"change_adapter\"]", %{email: %{adapter: "postmark"}})
+      |> form("form[phx-change=\"change_adapter\"]", %{email: %{adapter: "brevo"}})
       |> render_change()
 
       view
       |> form("form[phx-submit=\"save\"]", %{
-        email: %{adapter: "postmark", api_key: "def12345-abcd-1234-abcd-123456789def"}
+        email: %{adapter: "brevo", api_key: "xkeysib-def789ghi012"}
       })
       |> render_submit()
 
       refute Mailer.email_verified?()
     end
-
-    test "disconnecting clears verified flag", %{conn: conn} do
-      Settings.put_setting("email_adapter", "postmark")
-      Settings.put_secret("email_postmark_api_key", "pm_test_abc")
-      Mailer.mark_email_verified()
-      assert Mailer.email_verified?()
-
-      {:ok, view, _html} = live(conn, ~p"/admin/settings/email")
-
-      render_click(view, "disconnect")
-
-      refute Mailer.email_verified?()
-    end
   end
 
   describe "unauthenticated" do

test/berrypod_web/live/admin/settings_test.exs

@@ -240,6 +240,32 @@ defmodule BerrypodWeb.Admin.SettingsTest do
     end
   end
 
+  describe "from address" do
+    setup %{conn: conn, user: user} do
+      conn = log_in_user(conn, user)
+      %{conn: conn}
+    end
+
+    test "shows from address section", %{conn: conn} do
+      {:ok, _view, html} = live(conn, ~p"/admin/settings")
+
+      assert html =~ "From address"
+      assert html =~ "sender address"
+    end
+
+    test "saves from address", %{conn: conn} do
+      {:ok, view, _html} = live(conn, ~p"/admin/settings")
+
+      html =
+        view
+        |> form("form[phx-submit=\"save_from_address\"]", %{from_address: "shop@example.com"})
+        |> render_submit()
+
+      assert html =~ "From address saved"
+      assert Settings.get_setting("email_from_address") == "shop@example.com"
+    end
+  end
+
   describe "advanced section" do
     setup %{conn: conn, user: user} do
       conn = log_in_user(conn, user)