Refactor NotificationMailer to use parameterization (#25718)

app/mailers/notification_mailer.rb

@@ -1,83 +1,76 @@
 # frozen_string_literal: true
 
 class NotificationMailer < ApplicationMailer
-  helper :accounts
-  helper :statuses
+  helper :accounts,
+         :statuses,
+         :routing
 
-  helper RoutingHelper
+  before_action :process_params
+  before_action :set_status, only: [:mention, :favourite, :reblog]
+  before_action :set_account, only: [:follow, :favourite, :reblog, :follow_request]
 
-  def mention(recipient, notification)
-    @me     = recipient
-    @user   = recipient.user
-    @type   = 'mention'
-    @status = notification.target_status
+  default to: -> { email_address_with_name(@user.email, @me.username) }
 
+  def mention
     return unless @user.functional? && @status.present?
 
     locale_for_account(@me) do
       thread_by_conversation(@status.conversation)
-      mail to: email_address_with_name(@user.email, @me.username), subject: I18n.t('notification_mailer.mention.subject', name: @status.account.acct)
+      mail subject: default_i18n_subject(name: @status.account.acct)
     end
   end
 
-  def follow(recipient, notification)
-    @me      = recipient
-    @user    = recipient.user
-    @type    = 'follow'
-    @account = notification.from_account
-
+  def follow
     return unless @user.functional?
 
     locale_for_account(@me) do
-      mail to: email_address_with_name(@user.email, @me.username), subject: I18n.t('notification_mailer.follow.subject', name: @account.acct)
+      mail subject: default_i18n_subject(name: @account.acct)
     end
   end
 
-  def favourite(recipient, notification)
-    @me      = recipient
-    @user    = recipient.user
-    @type    = 'favourite'
-    @account = notification.from_account
-    @status  = notification.target_status
-
+  def favourite
     return unless @user.functional? && @status.present?
 
     locale_for_account(@me) do
       thread_by_conversation(@status.conversation)
-      mail to: email_address_with_name(@user.email, @me.username), subject: I18n.t('notification_mailer.favourite.subject', name: @account.acct)
+      mail subject: default_i18n_subject(name: @account.acct)
     end
   end
 
-  def reblog(recipient, notification)
-    @me      = recipient
-    @user    = recipient.user
-    @type    = 'reblog'
-    @account = notification.from_account
-    @status  = notification.target_status
-
+  def reblog
     return unless @user.functional? && @status.present?
 
     locale_for_account(@me) do
       thread_by_conversation(@status.conversation)
-      mail to: email_address_with_name(@user.email, @me.username), subject: I18n.t('notification_mailer.reblog.subject', name: @account.acct)
+      mail subject: default_i18n_subject(name: @account.acct)
     end
   end
 
-  def follow_request(recipient, notification)
-    @me      = recipient
-    @user    = recipient.user
-    @type    = 'follow_request'
-    @account = notification.from_account
-
+  def follow_request
     return unless @user.functional?
 
     locale_for_account(@me) do
-      mail to: email_address_with_name(@user.email, @me.username), subject: I18n.t('notification_mailer.follow_request.subject', name: @account.acct)
+      mail subject: default_i18n_subject(name: @account.acct)
     end
   end
 
   private
 
+  def process_params
+    @notification = params[:notification]
+    @me = params[:recipient]
+    @user = @me.user
+    @type = action_name
+  end
+
+  def set_status
+    @status = @notification.target_status
+  end
+
+  def set_account
+    @account = @notification.from_account
+  end
+
   def thread_by_conversation(conversation)
     return if conversation.nil?
 

app/services/notify_service.rb

@@ -162,7 +162,12 @@ class NotifyService < BaseService
   end
 
   def send_email!
-    NotificationMailer.public_send(@notification.type, @recipient, @notification).deliver_later(wait: 2.minutes) if NotificationMailer.respond_to?(@notification.type)
+    return unless NotificationMailer.respond_to?(@notification.type)
+
+    NotificationMailer
+      .with(recipient: @recipient, notification: @notification)
+      .public_send(@notification.type)
+      .deliver_later(wait: 2.minutes)
   end
 
   def email_needed?

spec/mailers/notification_mailer_spec.rb

@@ -23,7 +23,8 @@ RSpec.describe NotificationMailer do
 
   describe 'mention' do
     let(:mention) { Mention.create!(account: receiver.account, status: foreign_status) }
-    let(:mail) { described_class.mention(receiver.account, Notification.create!(account: receiver.account, activity: mention)) }
+    let(:notification) { Notification.create!(account: receiver.account, activity: mention) }
+    let(:mail) { prepared_mailer_for(receiver.account).mention }
 
     include_examples 'localized subject', 'notification_mailer.mention.subject', name: 'bob'
 
@@ -40,7 +41,8 @@ RSpec.describe NotificationMailer do
 
   describe 'follow' do
     let(:follow) { sender.follow!(receiver.account) }
-    let(:mail) { described_class.follow(receiver.account, Notification.create!(account: receiver.account, activity: follow)) }
+    let(:notification) { Notification.create!(account: receiver.account, activity: follow) }
+    let(:mail) { prepared_mailer_for(receiver.account).follow }
 
     include_examples 'localized subject', 'notification_mailer.follow.subject', name: 'bob'
 
@@ -56,7 +58,8 @@ RSpec.describe NotificationMailer do
 
   describe 'favourite' do
     let(:favourite) { Favourite.create!(account: sender, status: own_status) }
-    let(:mail) { described_class.favourite(own_status.account, Notification.create!(account: receiver.account, activity: favourite)) }
+    let(:notification) { Notification.create!(account: receiver.account, activity: favourite) }
+    let(:mail) { prepared_mailer_for(own_status.account).favourite }
 
     include_examples 'localized subject', 'notification_mailer.favourite.subject', name: 'bob'
 
@@ -73,7 +76,8 @@ RSpec.describe NotificationMailer do
 
   describe 'reblog' do
     let(:reblog) { Status.create!(account: sender, reblog: own_status) }
-    let(:mail) { described_class.reblog(own_status.account, Notification.create!(account: receiver.account, activity: reblog)) }
+    let(:notification) { Notification.create!(account: receiver.account, activity: reblog) }
+    let(:mail) { prepared_mailer_for(own_status.account).reblog }
 
     include_examples 'localized subject', 'notification_mailer.reblog.subject', name: 'bob'
 
@@ -90,7 +94,8 @@ RSpec.describe NotificationMailer do
 
   describe 'follow_request' do
     let(:follow_request) { Fabricate(:follow_request, account: sender, target_account: receiver.account) }
-    let(:mail) { described_class.follow_request(receiver.account, Notification.create!(account: receiver.account, activity: follow_request)) }
+    let(:notification) { Notification.create!(account: receiver.account, activity: follow_request) }
+    let(:mail) { prepared_mailer_for(receiver.account).follow_request }
 
     include_examples 'localized subject', 'notification_mailer.follow_request.subject', name: 'bob'
 
@@ -103,4 +108,10 @@ RSpec.describe NotificationMailer do
       expect(mail.body.encoded).to match('bob has requested to follow you')
     end
   end
+
+  private
+
+  def prepared_mailer_for(recipient)
+    described_class.with(recipient: recipient, notification: notification)
+  end
 end

spec/mailers/previews/notification_mailer_preview.rb

@@ -5,31 +5,40 @@
 class NotificationMailerPreview < ActionMailer::Preview
   # Preview this email at http://localhost:3000/rails/mailers/notification_mailer/mention
   def mention
-    m = Mention.last
-    NotificationMailer.mention(m.account, Notification.find_by(activity: m))
+    activity = Mention.last
+    mailer_for(activity.account, activity).mention
   end
 
   # Preview this email at http://localhost:3000/rails/mailers/notification_mailer/follow
   def follow
-    f = Follow.last
-    NotificationMailer.follow(f.target_account, Notification.find_by(activity: f))
+    activity = Follow.last
+    mailer_for(activity.target_account, activity).follow
   end
 
   # Preview this email at http://localhost:3000/rails/mailers/notification_mailer/follow_request
   def follow_request
-    f = Follow.last
-    NotificationMailer.follow_request(f.target_account, Notification.find_by(activity: f))
+    activity = Follow.last
+    mailer_for(activity.target_account, activity).follow_request
   end
 
   # Preview this email at http://localhost:3000/rails/mailers/notification_mailer/favourite
   def favourite
-    f = Favourite.last
-    NotificationMailer.favourite(f.status.account, Notification.find_by(activity: f))
+    activity = Favourite.last
+    mailer_for(activity.status.account, activity).favourite
   end
 
   # Preview this email at http://localhost:3000/rails/mailers/notification_mailer/reblog
   def reblog
-    r = Status.where.not(reblog_of_id: nil).first
-    NotificationMailer.reblog(r.reblog.account, Notification.find_by(activity: r))
+    activity = Status.where.not(reblog_of_id: nil).first
+    mailer_for(activity.reblog.account, activity).reblog
+  end
+
+  private
+
+  def mailer_for(account, activity)
+    NotificationMailer.with(
+      recipient: account,
+      notification: Notification.find_by(activity: activity)
+    )
   end
 end