Refactor NotificationMailer to use parameterization (mastodon#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 @@ def subscribed_to_web_push?
  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 @@
 
  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 @@
 
  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 @@
 
  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 @@
 
  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 @@
 
  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 @@
  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