Move email sending to foreground, surface delivery error in the UI

lib/asciinema.ex

@@ -32,9 +32,8 @@ defmodule Asciinema do
 
   def send_account_deletion_email(user) do
     token = Accounts.generate_deletion_token(user)
-    Emails.send_email(:account_deletion, user.email, token)
 
-    :ok
+    Emails.send_email(:account_deletion, user.email, token)
   end
 
   defdelegate verify_login_token(token), to: Accounts

lib/asciinema/emails.ex

@@ -1,47 +1,33 @@
 defmodule Asciinema.Emails do
   alias Asciinema.Emails.{Email, Mailer}
 
-  defmodule Job do
-    use Oban.Worker, queue: :emails
-
-    @impl Oban.Worker
-    def perform(job) do
-      case job.args["type"] do
-        "signup" ->
-          job.args["to"]
-          |> Email.signup_email(job.args["token"])
-          |> deliver()
-
-        "login" ->
-          job.args["to"]
-          |> Email.login_email(job.args["token"])
-          |> deliver()
-
-        "account_deletion" ->
-          job.args["to"]
-          |> Email.account_deletion_email(job.args["token"])
-          |> deliver()
-      end
-
-      :ok
-    end
-
-    defp deliver(email) do
-      with {:permanent_failure, _, _} <- Mailer.deliver_now!(email) do
-        {:cancel, :permanent_failure}
-      end
-    end
+  def send_email(:signup, to, token) do
+    to
+    |> Email.signup_email(token)
+    |> deliver()
   end
 
-  def send_email(type, to, token) do
-    Oban.insert!(Job.new(%{type: type, to: to, token: token}))
+  def send_email(:login, to, token) do
+    to
+    |> Email.login_email(token)
+    |> deliver()
+  end
 
-    :ok
+  def send_email(:account_deletion, to, token) do
+    to
+    |> Email.account_deletion_email(token)
+    |> deliver()
   end
 
   def send_email(:test, to) do
     to
     |> Email.test_email()
-    |> Mailer.deliver_now!()
+    |> deliver()
+  end
+
+  defp deliver(email) do
+    with {:ok, _email} <- Mailer.deliver_now(email) do
+      :ok
+    end
   end
 end

lib/asciinema_web/controllers/login/sent.html.heex

@@ -15,7 +15,7 @@
 
       <p>
         If the email doesn't show up in your inbox, check your spam folder,
-        or <a href={~p"/login/new"}>try again</a>.
+        or <a href={~p"/login/new"}>try again</a> in a moment.
       </p>
     </div>
   </div>

lib/asciinema_web/controllers/login_controller.ex

@@ -9,7 +9,7 @@ defmodule AsciinemaWeb.LoginController do
   end
 
   def create(%{assigns: %{bot: true}} = conn, params) do
-    Logger.warn("bot login attempt: #{inspect(params)}")
+    Logger.warning("bot login attempt: #{inspect(params)}")
 
     redirect(conn, to: ~p"/login/sent")
   end
@@ -35,6 +35,11 @@ defmodule AsciinemaWeb.LoginController do
 
       {:error, :email_missing} ->
         redirect(conn, to: ~p"/login/sent")
+
+      {:error, reason} ->
+        Logger.warning("email delivery error: #{inspect(reason)}")
+
+        render(conn, :new, error: "Error sending email, please try again later.")
     end
   end
 

lib/asciinema_web/controllers/user_controller.ex

@@ -2,6 +2,7 @@ defmodule AsciinemaWeb.UserController do
   use AsciinemaWeb, :controller
   alias Asciinema.{Accounts, Streaming, Recordings}
   alias AsciinemaWeb.Auth
+  require Logger
 
   plug :require_current_user when action in [:edit, :update]
 
@@ -152,10 +153,18 @@ defmodule AsciinemaWeb.UserController do
     user = conn.assigns.current_user
     address = user.email
 
-    :ok = Asciinema.send_account_deletion_email(user)
+    case Asciinema.send_account_deletion_email(user) do
+      :ok ->
+        conn
+        |> put_flash(:info, "Account removal initiated - check your inbox (#{address})")
+        |> redirect(to: profile_path(conn))
 
-    conn
-    |> put_flash(:info, "Account removal initiated - check your inbox (#{address})")
-    |> redirect(to: profile_path(conn))
+      {:error, reason} ->
+        Logger.warning("email delivery error: #{inspect(reason)}")
+
+        conn
+        |> put_flash(:error, "Error sending email, please try again later")
+        |> redirect(to: ~p"/user/edit")
+    end
   end
 end

test/asciinema_test.exs

@@ -1,5 +1,6 @@
 defmodule AsciinemaTest do
   import Asciinema.Factory
+  import Bamboo.Test
   use Asciinema.DataCase
   use Oban.Testing, repo: Asciinema.Repo
   alias Asciinema.Recordings
@@ -28,48 +29,42 @@ defmodule AsciinemaTest do
 
       assert Asciinema.send_login_email("TEST@EXAMPLE.COM") == :ok
 
-      assert_enqueued(
-        worker: Asciinema.Emails.Job,
-        args: %{"type" => "login", "to" => "test@example.com"}
-      )
+      assert_email_delivered_with(to: [{nil, "test@example.com"}], subject: "Login to localhost")
     end
 
     test "existing user, by username" do
       insert(:user, username: "foobar", email: "foobar123@example.com")
 
       assert Asciinema.send_login_email("foobar") == :ok
 
-      assert_enqueued(
-        worker: Asciinema.Emails.Job,
-        args: %{"type" => "login", "to" => "foobar123@example.com"}
+      assert_email_delivered_with(
+        to: [{nil, "foobar123@example.com"}],
+        subject: "Login to localhost"
       )
     end
 
     test "non-existing user, by email" do
       assert Asciinema.send_login_email("NEW@EXAMPLE.COM") == :ok
 
-      assert_enqueued(
-        worker: Asciinema.Emails.Job,
-        args: %{"type" => "signup", "to" => "new@example.com"}
-      )
+      assert_email_delivered_with(to: [{nil, "new@example.com"}], subject: "Welcome to localhost")
     end
 
     test "non-existing user, by email, when sign up is disabled" do
       assert Asciinema.send_login_email("new@example.com", false) == {:error, :user_not_found}
 
-      refute_enqueued(worker: Asciinema.Emails.Job)
+      assert_no_emails_delivered()
     end
 
     test "non-existing user, by email, when email is invalid" do
       assert Asciinema.send_login_email("new@") == {:error, :email_invalid}
 
-      refute_enqueued(worker: Asciinema.Emails.Job)
+      assert_no_emails_delivered()
     end
 
     test "non-existing user, by username" do
       assert Asciinema.send_login_email("idontexist") == {:error, :user_not_found}
 
-      refute_enqueued(worker: Asciinema.Emails.Job)
+      assert_no_emails_delivered()
     end
   end
 

test/controllers/login_controller_test.exs

@@ -1,41 +1,47 @@
 defmodule Asciinema.LoginControllerTest do
   use AsciinemaWeb.ConnCase
   use Oban.Testing, repo: Asciinema.Repo
+  import Bamboo.Test
 
   @honeypot_detection_header "x-melliculum"
 
   test "with valid email", %{conn: conn} do
     conn = post conn, "/login", %{login: %{email: "new@example.com", username: ""}}
+
     assert redirected_to(conn, 302) == "/login/sent"
     assert get_resp_header(conn, @honeypot_detection_header) == []
-    assert_enqueued(worker: Asciinema.Emails.Job, args: %{"to" => "new@example.com"})
+    assert_email_delivered_with(to: [{nil, "new@example.com"}])
   end
 
   test "with valid email (uppercase)", %{conn: conn} do
     conn = post conn, "/login", %{login: %{email: "NEW@EXAMPLE.COM", username: ""}}
+
     assert redirected_to(conn, 302) == "/login/sent"
     assert get_resp_header(conn, @honeypot_detection_header) == []
-    assert_enqueued(worker: Asciinema.Emails.Job, args: %{"to" => "new@example.com"})
+    assert_email_delivered_with(to: [{nil, "new@example.com"}])
   end
 
   test "with invalid email", %{conn: conn} do
     conn = post conn, "/login", %{login: %{email: "new@", username: ""}}
+
     assert html_response(conn, 200) =~ "correct email"
     assert get_resp_header(conn, @honeypot_detection_header) == []
-    refute_enqueued(worker: Asciinema.Emails.Job)
+    assert_no_emails_delivered()
   end
 
   test "as bot with username", %{conn: conn} do
     conn = post conn, "/login", %{login: %{email: "bot@example.com", username: "bot"}}
+
     assert redirected_to(conn, 302) == "/login/sent"
     assert List.first(get_resp_header(conn, @honeypot_detection_header)) == "machina"
-    refute_enqueued(worker: Asciinema.Emails.Job)
+    assert_no_emails_delivered()
   end
 
   test "as bot with terms", %{conn: conn} do
     conn = post conn, "/login", %{login: %{email: "bot@example.com", username: "", terms: "1"}}
+
     assert redirected_to(conn, 302) == "/login/sent"
     assert List.first(get_resp_header(conn, @honeypot_detection_header)) == "machina"
-    refute_enqueued(worker: Asciinema.Emails.Job)
+    assert_no_emails_delivered()
   end
 end

test/controllers/user_controller_test.exs

@@ -2,6 +2,7 @@ defmodule Asciinema.UserControllerTest do
   use AsciinemaWeb.ConnCase
   use Oban.Testing, repo: Asciinema.Repo
   import Asciinema.Factory
+  import Bamboo.Test
   alias Asciinema.Accounts
 
   describe "sign-up" do
@@ -133,14 +134,14 @@ defmodule Asciinema.UserControllerTest do
 
   describe "account deletion" do
     test "phase 1", %{conn: conn} do
-      user = insert(:user)
+      user = insert(:user, email: "test@example.com")
       conn = log_in(conn, user)
 
       conn = delete(conn, ~p"/user")
 
       assert response(conn, 302)
       assert flash(conn, :info) =~ ~r/initiated/i
-      assert_enqueued(worker: Asciinema.Emails.Job, args: %{"type" => "account_deletion"})
+      assert_email_delivered_with(to: [{nil, "test@example.com"}], subject: "Account deletion")
     end
 
     test "phase 2", %{conn: conn} do