Repository: phoenixframework/phoenix
Pull request #4373 (2021-07-16 22:13:13)

Add swoosh integration to phx.gen.auth generator
This commit adds the integration of the new mailer that uses Swoosh to
the existing `phx.gen.auth` generator. It also adds instructions for the
mailer installation if the app does not define the mailer module.

philss: ping @josevalim :)
rickclare: Perhaps the following is more of a discussion point than something that in the scope of this PR, but:

As the act of sending production emails (via Mailgun, Sendgrid, etc) can take significantly more time than regular Controller actions and Ecto operations, should any thought be given to using asynchronous email delivery as part of these new email generators? (e.g. as described in the Swoosh documentation https://github.com/swoosh/swoosh#async-emails).

In my Ruby-on-Rails days, we always found that sending email *synchronously* (eg during a user registration process) considerably degraded the user experience - HTTP requests that had an email-sending side effect were orders of magnitude slower, leading to user annoyance. On such projects we generally resorted to using solutions such as Redis-backed async queue (hopefully not necessary with Elixir).

I'm unsure of the best place location at which one might wrap mail delivery in something such as a `Task.async/1` call (eg whether at either Controller, Context or Mailer level?). Perhaps we can draw on advice from more seasoned Elixir engineers than myself on this?

josevalim: If you are using Mailgun or Sendgrid, sending an email is relatively fast (< 100ms), as they don't block until the email is effectively sent. I would revisit this if using gen_smtp though. Plus, in Rails, you have to avoid blocking the request process as it may block other requests in the system, and that's not a concern here. So for now, my answer would be that they don't need to be made async. :)
rickclare: That makes good sense (and is very reassuring). Thanks for the explanation Jose.
ZombieHarvester: @philss Why Swoosh and not Bamboo? I'm using Bamboo, I wonder why Swoosh is chosen, is it any better than Bamboo?
josevalim: @ZombieHarvester they are pretty much equivalent from our point of view too. We had more experience with Swoosh, based on [Bytepack](https://github.com/dashbitco/bytepack_archive), so that's what we went with. :)
philss: Sorry about the CI :disappointed: 
I always forget to run integration tests locally.
josevalim: @philss don't worry about the integration tests :) I let the CI do the work most of the time for me :)

Patch:
diff --git a/lib/mix/tasks/phx.gen.auth.ex b/lib/mix/tasks/phx.gen.auth.ex
index c9422ba25..7ffba1d28 100644
--- a/lib/mix/tasks/phx.gen.auth.ex
+++ b/lib/mix/tasks/phx.gen.auth.ex
@@ -143,6 +143,7 @@ defmodule Mix.Tasks.Phx.Gen.Auth do
     |> maybe_inject_router_import(binding)
     |> maybe_inject_router_plug()
     |> maybe_inject_app_layout_menu()
+    |> Gen.Notifier.maybe_print_mailer_installation_instructions()
     |> print_shell_instructions()
   end
 
diff --git a/lib/mix/tasks/phx.gen.notifier.ex b/lib/mix/tasks/phx.gen.notifier.ex
index 3f68d735b..886e2dd87 100644
--- a/lib/mix/tasks/phx.gen.notifier.ex
+++ b/lib/mix/tasks/phx.gen.notifier.ex
@@ -11,8 +11,13 @@ defmodule Mix.Tasks.Phx.Gen.Notifier do
   are the functions that will be created prefixed by "deliver",
   so the message name should be "snake_case" without punctuation.
 
-  Note that this task expects that you have a mailer defined
-  as "YourApp.Mailer". Check the `mix phx.new` task for more details.
+  Additionaly a context app can be specified with the flag
+  `--context-app`, which is useful if the notifier is being
+  generated in a different app under an umbrella.
+
+      mix phx.gen.notifier Accounts User welcome_user --context-app marketing
+
+  The app "marketing" must exist before the command is executed.
   """
 
   use Mix.Task
@@ -49,12 +54,19 @@ defmodule Mix.Tasks.Phx.Gen.Notifier do
 
     prompt_for_conflicts(context)
 
-    copy_new_files(context, binding, paths)
+    if "--no-compile" not in args do
+      Mix.Task.run("compile")
+    end
+
+    context
+    |> copy_new_files(binding, paths)
+    |> maybe_print_mailer_installation_instructions()
   end
 
   @doc false
   def build(args, help \\ __MODULE__) do
     {opts, parsed, _} = parse_opts(args)
+
     [context_name, notifier_name | notifier_messages] = validate_args!(parsed, help)
 
     notifier_module = inspect(Module.concat(context_name, "#{notifier_name}Notifier"))
@@ -164,4 +176,44 @@ defmodule Mix.Tasks.Phx.Gen.Notifier do
     |> files_to_be_generated()
     |> Mix.Phoenix.prompt_for_conflicts()
   end
+
+  @doc """
+  Print mailer instructions if mailer is not defined.
+
+  This is useful for applications there were created without the
+  mailer.
+  """
+  @spec maybe_print_mailer_installation_instructions(%Context{}) :: %Context{}
+  def maybe_print_mailer_installation_instructions(%Context{} = context) do
+    mailer_module = Module.concat([context.base_module, "Mailer"])
+
+    unless Code.ensure_loaded?(mailer_module) do
+      Mix.shell().info("""
+      Unable to find the "#{inspect(mailer_module)}" module defined.
+
+      A mailer module like the following is expected to be defined
+      in your application in order to send emails.
+
+          defmodule #{inspect(mailer_module)} do
+            use Swoosh.Mailer, otp_app: #{inspect(context.context_app)}
+          end
+
+      It is also necessary to add "swoosh" as a dependency in your
+      "mix.exs" file:
+
+          def deps do
+            [{:swoosh, "~> 1.4"}]
+          end
+
+      Finally, an adapter needs to be set in your configuration:
+
+          import Config
+          config #{inspect(context.context_app)}, #{inspect(mailer_module)}, adapter: Swoosh.Adapters.Local
+
+      Check https://hexdocs.pm/swoosh for more details.
+      """)
+    end
+
+    context
+  end
 end
diff --git a/priv/templates/phx.gen.auth/context_fixtures_functions.ex b/priv/templates/phx.gen.auth/context_fixtures_functions.ex
index 5235b0229..5a8b1474e 100644
--- a/priv/templates/phx.gen.auth/context_fixtures_functions.ex
+++ b/priv/templates/phx.gen.auth/context_fixtures_functions.ex
@@ -18,7 +18,7 @@
   end
 
   def extract_<%= schema.singular %>_token(fun) do
-    {:ok, captured} = fun.(&"[TOKEN]#{&1}[TOKEN]")
-    [_, token | _] = String.split(captured.body, "[TOKEN]")
+    {:ok, captured_email} = fun.(&"[TOKEN]#{&1}[TOKEN]")
+    [_, token | _] = String.split(captured_email.text_body, "[TOKEN]")
     token
   end
diff --git a/priv/templates/phx.gen.auth/notifier.ex b/priv/templates/phx.gen.auth/notifier.ex
index b210416a2..28ce046cd 100644
--- a/priv/templates/phx.gen.auth/notifier.ex
+++ b/priv/templates/phx.gen.auth/notifier.ex
@@ -1,21 +1,27 @@
 defmodule <%= inspect context.module %>.<%= inspect schema.alias %>Notifier do
-  # For simplicity, this module simply logs messages to the terminal.
-  # You should replace it by a proper email or notification tool, such as:
-  #
-  #   * Swoosh - https://hexdocs.pm/swoosh
-  #   * Bamboo - https://hexdocs.pm/bamboo
-  #
-  defp deliver(to, body) do
-    require Logger
-    Logger.debug(body)
-    {:ok, %{to: to, body: body}}
+  import Swoosh.Email
+
+  alias <%= inspect context.base_module %>.Mailer
+
+  # Delivers the email using the application mailer.
+  defp deliver(recipient, subject, body) do
+    email =
+      new()
+      |> to(recipient)
+      |> from({"MyApp", "contact@example.com"})
+      |> subject(subject)
+      |> text_body(body)
+
+    with {:ok, _metadata} <- Mailer.deliver(email) do
+      {:ok, email}
+    end
   end
 
   @doc """
   Deliver instructions to confirm account.
   """
   def deliver_confirmation_instructions(<%= schema.singular %>, url) do
-    deliver(<%= schema.singular %>.email, """
+    deliver(<%= schema.singular %>.email, "Confirmation instructions", """
 
     ==============================
 
@@ -35,7 +41,7 @@ defmodule <%= inspect context.module %>.<%= inspect schema.alias %>Notifier do
   Deliver instructions to reset a <%= schema.singular %> password.
   """
   def deliver_reset_password_instructions(<%= schema.singular %>, url) do
-    deliver(<%= schema.singular %>.email, """
+    deliver(<%= schema.singular %>.email, "Reset password instructions", """
 
     ==============================
 
@@ -55,7 +61,7 @@ defmodule <%= inspect context.module %>.<%= inspect schema.alias %>Notifier do
   Deliver instructions to update a <%= schema.singular %> email.
   """
   def deliver_update_email_instructions(<%= schema.singular %>, url) do
-    deliver(<%= schema.singular %>.email, """
+    deliver(<%= schema.singular %>.email, "Update email instructions", """
 
     ==============================
 


Test patch:
diff --git a/test/mix/tasks/phx.gen.auth_test.exs b/test/mix/tasks/phx.gen.auth_test.exs
index 3b4736d7d..57436badb 100644
--- a/test/mix/tasks/phx.gen.auth_test.exs
+++ b/test/mix/tasks/phx.gen.auth_test.exs
@@ -89,7 +89,14 @@ defmodule Mix.Tasks.Phx.Gen.AuthTest do
       assert_file "lib/my_app/accounts.ex"
       assert_file "lib/my_app/accounts/user.ex"
       assert_file "lib/my_app/accounts/user_token.ex"
-      assert_file "lib/my_app/accounts/user_notifier.ex"
+      assert_file "lib/my_app/accounts/user_notifier.ex", fn file ->
+        assert file =~ "defmodule MyApp.Accounts.UserNotifier do"
+        assert file =~ "import Swoosh.Email"
+        assert file =~ "Mailer.deliver(email)"
+        assert file =~ ~s|deliver(user.email, "Confirmation instructions",|
+        assert file =~ ~s|deliver(user.email, "Reset password instructions",|
+        assert file =~ ~s|deliver(user.email, "Update email instructions",|
+      end
       assert_file "test/my_app/accounts_test.exs"
       assert_file "test/support/fixtures/accounts_fixtures.ex"
       assert_file "lib/my_app_web/controllers/user_auth.ex"
@@ -187,6 +194,14 @@ defmodule Mix.Tasks.Phx.Gen.AuthTest do
 
         $ mix ecto.migrate
       """]}
+
+      assert_received {:mix_shell, :info, ["Unable to find the \"MyApp.Mailer\"" <> mailer_notice]}
+      assert mailer_notice =~ ~s(A mailer module like the following is expected to be defined)
+      assert mailer_notice =~ ~s(in your application in order to send emails.)
+      assert mailer_notice =~ ~s(defmodule MyApp.Mailer do)
+      assert mailer_notice =~ ~s(use Swoosh.Mailer, otp_app: :my_app)
+      assert mailer_notice =~ ~s(def deps do)
+      assert mailer_notice =~ ~s(https://hexdocs.pm/swoosh)
     end)
   end
 
diff --git a/test/mix/tasks/phx.gen.notifier_test.exs b/test/mix/tasks/phx.gen.notifier_test.exs
index a0fa1ddbf..1ad27d93b 100644
--- a/test/mix/tasks/phx.gen.notifier_test.exs
+++ b/test/mix/tasks/phx.gen.notifier_test.exs
@@ -1,5 +1,8 @@
 Code.require_file("../../../installer/test/mix_helper.exs", __DIR__)
 
+defmodule MyTestApp.Mailer do
+end
+
 defmodule Mix.Tasks.Phx.Gen.NotifierTest do
   use ExUnit.Case
   import MixHelper
@@ -12,7 +15,7 @@ defmodule Mix.Tasks.Phx.Gen.NotifierTest do
 
   test "new notifier", config do
     in_tmp_project(config.test, fn ->
-      Gen.Notifier.run(~w(Accounts User welcome_user reset_password))
+      Gen.Notifier.run(~w(Accounts User welcome_user reset_password --no-compile))
 
       assert_file("lib/phoenix/accounts/user_notifier.ex", fn file ->
         assert file =~ ~S|defmodule Phoenix.Accounts.UserNotifier do|
@@ -36,7 +39,7 @@ defmodule Mix.Tasks.Phx.Gen.NotifierTest do
       send(self(), {:mix_shell_input, :yes?, true})
       send(self(), {:mix_shell_input, :yes?, true})
 
-      Gen.Notifier.run(~w(Accounts User account_confirmation))
+      Gen.Notifier.run(~w(Accounts User account_confirmation --no-compile))
 
       assert_received {:mix_shell, :info,
                        ["The following files conflict with new files to be generated:" <> notice]}
@@ -64,7 +67,7 @@ defmodule Mix.Tasks.Phx.Gen.NotifierTest do
 
   test "generates nested notifier", config do
     in_tmp_project(config.test, fn ->
-      Gen.Notifier.run(~w(Admin.Accounts User welcome_user reset_password))
+      Gen.Notifier.run(~w(Admin.Accounts User welcome_user reset_password --no-compile))
 
       assert_file("lib/phoenix/admin/accounts/user_notifier.ex", fn file ->
         assert file =~ ~S|defmodule Phoenix.Admin.Accounts.UserNotifier do|
@@ -85,7 +88,7 @@ defmodule Mix.Tasks.Phx.Gen.NotifierTest do
   test "in an umbrella with a context_app, generates the notifier", config do
     in_tmp_umbrella_project(config.test, fn ->
       Application.put_env(:phoenix, :generators, context_app: {:another_app, "another_app"})
-      Gen.Notifier.run(~w(Accounts User welcome_user reset_password))
+      Gen.Notifier.run(~w(Accounts User welcome_user reset_password --no-compile))
 
       assert_file("another_app/lib/another_app/accounts/user_notifier.ex", fn file ->
         assert file =~ ~S|defmodule AnotherApp.Accounts.UserNotifier do|
@@ -100,30 +103,53 @@ defmodule Mix.Tasks.Phx.Gen.NotifierTest do
   test "invalid mix arguments", config do
     in_tmp_project(config.test, fn ->
       assert_raise Mix.Error, ~r/Expected the context, "blog", to be a valid module name/, fn ->
-        Gen.Notifier.run(~w(blog Post new_post))
+        Gen.Notifier.run(~w(blog Post new_post --no-compile))
       end
 
       assert_raise Mix.Error, ~r/Expected the notifier, "posts", to be a valid module name/, fn ->
-        Gen.Notifier.run(~w(Post posts new_post))
+        Gen.Notifier.run(~w(Post posts new_post --no-compile))
       end
 
       assert_raise Mix.Error,
                    ~r/Cannot generate context Phoenix because it has the same name as the application/,
                    fn ->
-                     Gen.Notifier.run(~w(Phoenix Post new_blog_post))
+                     Gen.Notifier.run(~w(Phoenix Post new_blog_post --no-compile))
                    end
 
       assert_raise Mix.Error,
                    ~r/Cannot generate notifier Phoenix because it has the same name as the application/,
                    fn ->
-                     Gen.Notifier.run(~w(Blog Phoenix new_blog_post))
+                     Gen.Notifier.run(~w(Blog Phoenix new_blog_post --no-compile))
                    end
 
       assert_raise Mix.Error,
                    ~r/Cannot generate notifier "Post" because one of the messages is invalid: "NewPost"/,
                    fn ->
-                     Gen.Notifier.run(~w(Blog Post NewPost))
+                     Gen.Notifier.run(~w(Blog Post NewPost --no-compile))
                    end
     end)
   end
+
+  test "maybe_print_mailer_installation_instructions/1", config do
+    in_tmp_project(config.test, fn ->
+      context = Mix.Phoenix.Context.new(MyContext.UserNotifier, [])
+
+      Gen.Notifier.maybe_print_mailer_installation_instructions(context)
+
+      assert_received {:mix_shell, :info, ["Unable to find the \"Phoenix.Mailer\"" <> notice]}
+
+      assert notice =~ ~s(A mailer module like the following is expected to be defined)
+      assert notice =~ ~s(in your application in order to send emails.)
+      assert notice =~ ~s(defmodule Phoenix.Mailer do)
+      assert notice =~ ~s(use Swoosh.Mailer, otp_app: :phoenix)
+      assert notice =~ ~s(def deps do)
+      assert notice =~ ~s(https://hexdocs.pm/swoosh)
+
+      context_with_mailer = %{context | base_module: MyTestApp}
+
+      Gen.Notifier.maybe_print_mailer_installation_instructions(context_with_mailer)
+
+      refute_received {:mix_shell, :info, _info}
+    end)
+  end
 end