メールを送信する処理をどこに書くべきか？

[willnet]

Railsのきれいなコードのお題案 · Issue #1 · clean-rails-ja/conversationからの転載

例

転職サイトでユーザーが求人に応募したとき、下記の3つのメールを送信したい。

• ユーザーには「応募しました」のメール
  • ただし、初回の応募には注意事項に関する説明をつける
• 企業には「応募がありました」のメール
• 運営には「応募がありました」のメール

実装方法の例

Controller 内で愚直に呼び出した例は下記の通り。

# app/controllers/entries_controller.rb
class EntriesController < ApplicationController
  def create
    if @entry.save
      AdminMailer.new_entry(@entry).deliver_later
      CompanyMailer.new_entry(@entry).deliver_later
      if current_user.entries.count.zero?
        UserMailer.new_entry_with_description(@entry).deliver_later
      else
        UserMailer.new_entry(@entry).deliver_later
      end
    else
      render :new
    end
  end
end

これはちょっと微妙なコードですが、これを改善しようとした場合、どこにコードを置くのが綺麗なのか？

本題とは別の話ですが…

そもそも、Mailer のクラス名やメソッド名はどのようにするのが綺麗なのか。

[yatmsu]

# app/controllers/entries_controller.rb
UserMailer.new_entry(@entry, description: current_user.entries.count.zero?).deliver_later

この程度の内容であればMailerClassにオプションで渡してしまっていいと思いますが、もっと複雑な分岐がある場合はPOROも検討・・・ですかね。
基本的にメール送信はControllerに書いてしまいたい派。初見でも分かりやすくなるので。

[willnet]

基本的にPOROでServiceオブジェクトのようなもの(別にServiceという名前でなくてもいいんだけど)を作る、という方針がいいんじゃないかなーと思いました。

コード例

class DeliverAfterCreateEntryService
  def self.call(user:, entry:)
    new(user: user, entry: entry).call
  end

  def initialize(user:, entry:)
    @user = user
    @entry = entry
  end

  def call
    deliver_to_admin
    deliver_to_company
    deliver_to_user
  end

  private

  attr_reader :user, :entry

  def deliver_to_admin
    AdminMailer.new_entry(entry).deliver_later
  end

  def deliver_to_company
    CompanyMailer.new_entry(entry).deliver_later
  end

  def deliver_to_user
    if initial_entry?
      UserMailer.new_entry_with_description(entry).deliver_later
    else
      UserMailer.new_entry(entry).deliver_later
    end
  end

  def initial_entry?
    user.entries.count.zero? # 条件的にちょっとおかしい気がするけど元のコードを尊重
  end
end

class EntriesController < ApplicationController
  def create
    if @entry.save
      DeliverAfterCreateEntryService.call(user: current_user, entry: @entry)
    else
      render :new
    end
  end
end

[willnet]

@willnet

そもそも、Mailer のクラス名やメソッド名はどのようにするのが綺麗なのか

これは僕も気になります…。何も考えずに書くとUserMailerがとてもFatになる印象

[ttanimichi]

めっちゃ異論が出ると思いますが、after_action に書きたい派です。controller の callbacks 嫌われてますよね。。
メール通知とか Slack 通知とかロギングとかってビジネスロジックではないというか一級市民ではないというか副作用に過ぎないと思っていて（だって最悪こいつらって動かなくてもシステムとしては動作しますよね？）、なので action から追い出したいというか、AOP 的な感じで済ませたいので、AOP といえば Rails の場合、after_action とかの callbacks になるかなって。あるいは after_action から PORO を呼ぶかもしれないです。
逆に僕はそういったことにしか callbacks は使わないようにしてます。

[hanachin]

初回の求人応募と2回め以降の求人応募に違いがあるならメールを送る段階で初回かどうかで出し分けるよりもモデル自体を分けたほうがよさそう(同じテーブルに永続化するにしても)。

# app/controllers/entries_controller.rb
class EntriesController < ApplicationController
  class Entry < ::Entry
    after_create do
      AdminMailer.new_entry(self).deliver_later
      CompanyMailer.new_entry(self).deliver_later
      UserMailer.new_entry(self).deliver_later
    end
  end

  class InitialEntry < ::Entry
    self.table_name = "entries"
    after_create do
      AdminMailer.new_entry(self).deliver_later
      CompanyMailer.new_entry(self).deliver_later
      UserMailer.new_entry_with_description(self).deliver_later
    end
  end

  def create
    if current_user.entries.count.zero?
      @entry = InitialEntry.new(new_entry_params)
    else
      @entry = Entry.new(new_entry_params)
    end

    unless @entry.save
      render :new and return
    end
  end
end

追記

Twitterの方でやりとりしていました。

最初のエントリと2回め以降のエントリでモデル自体を分けるならいっそコントローラ自体分けたほうがよさそうにも思えます。
初回の求人応募の場合のみ特定のページにリダイレクトなど、メール送信以外でも動作を分ける必要が出てくる可能性がありそうです。

https://twitter.com/Arista_999/status/987130013488054273

はなちん

https://twitter.com/Arista_999/status/987800461745901568

例えばこういう感じでroutesから分けてしまってもいいかもしれません

# config/routes.rb

post "/entries/1", to: 'initial_entry#create'
resources :entries

# app/controllers/initial_entry_controller.rb
class InitialEntryController < ApplicationController
  class InitialEntry < ::Entry
    self.table_name = "entries"
    after_create do
      AdminMailer.new_entry(self).deliver_later
      CompanyMailer.new_entry(self).deliver_later
      UserMailer.new_entry_with_description(self).deliver_later
    end
  end

  before_action do
    if current_user.entries.count.nonzero?
      raise
    end
  end

  def create
    @entry = InitialEntry.new(new_entry_params)
    if @entry.save
      redirect_to survey_path
    else
      render :new
    end
  end
end

# app/controllers/entries_controller.rb
class EntriesController < ApplicationController
  class Entry < ::Entry
    after_create do
      AdminMailer.new_entry(self).deliver_later
      CompanyMailer.new_entry(self).deliver_later
      UserMailer.new_entry(self).deliver_later
    end
  end

  before_action do
    if current_user.entries.count.zero?
      raise
    end
  end

  def create
    if @entry.save
      redirect_to entry_path(@entry)
    else
      render :new
    end
  end
end

コントローラ自体を分けてしまえば初回応募時と2回め以降の応募時の要求の違いが増えても、分岐を増やさず各コントローラに追記するだけで対応できそうです。

[ttanimichi]

Service ならまだしも AR::Base な Model がメール通知みたいな副作用と絡むと魔物が生まれそうです