Misc spec/refactor to user mailer and user mailer spec (#27486)

app/controllers/admin/disputes/appeals_controller.rb

@@ -20,7 +20,7 @@ class Admin::Disputes::AppealsController < Admin::BaseController
     authorize @appeal, :approve?
     log_action :reject, @appeal
     @appeal.reject!(current_account)
-    UserMailer.appeal_rejected(@appeal.account.user, @appeal)
+    UserMailer.appeal_rejected(@appeal.account.user, @appeal).deliver_later
     redirect_to disputes_strike_path(@appeal.strike)
   end
 

app/mailers/user_mailer.rb

@@ -8,13 +8,15 @@ class UserMailer < Devise::Mailer
   helper :instance
   helper :statuses
   helper :formatting
+  helper :routing
 
-  helper RoutingHelper
+  before_action :set_instance
+
+  default to: -> { @resource.email }
 
   def confirmation_instructions(user, token, *, **)
     @resource = user
     @token    = token
-    @instance = Rails.configuration.x.local_domain
 
     return unless @resource.active_for_authentication?
 
@@ -28,185 +30,177 @@ class UserMailer < Devise::Mailer
   def reset_password_instructions(user, token, *, **)
     @resource = user
     @token    = token
-    @instance = Rails.configuration.x.local_domain
 
     return unless @resource.active_for_authentication?
 
     I18n.with_locale(locale) do
-      mail to: @resource.email, subject: I18n.t('devise.mailer.reset_password_instructions.subject')
+      mail subject: default_devise_subject
     end
   end
 
   def password_change(user, *, **)
     @resource = user
-    @instance = Rails.configuration.x.local_domain
 
     return unless @resource.active_for_authentication?
 
     I18n.with_locale(locale) do
-      mail to: @resource.email, subject: I18n.t('devise.mailer.password_change.subject')
+      mail subject: default_devise_subject
     end
   end
 
   def email_changed(user, *, **)
     @resource = user
-    @instance = Rails.configuration.x.local_domain
 
     return unless @resource.active_for_authentication?
 
     I18n.with_locale(locale) do
-      mail to: @resource.email, subject: I18n.t('devise.mailer.email_changed.subject')
+      mail subject: default_devise_subject
     end
   end
 
   def two_factor_enabled(user, *, **)
     @resource = user
-    @instance = Rails.configuration.x.local_domain
 
     return unless @resource.active_for_authentication?
 
     I18n.with_locale(locale) do
-      mail to: @resource.email, subject: I18n.t('devise.mailer.two_factor_enabled.subject')
+      mail subject: default_devise_subject
     end
   end
 
   def two_factor_disabled(user, *, **)
     @resource = user
-    @instance = Rails.configuration.x.local_domain
 
     return unless @resource.active_for_authentication?
 
     I18n.with_locale(locale) do
-      mail to: @resource.email, subject: I18n.t('devise.mailer.two_factor_disabled.subject')
+      mail subject: default_devise_subject
     end
   end
 
   def two_factor_recovery_codes_changed(user, *, **)
     @resource = user
-    @instance = Rails.configuration.x.local_domain
 
     return unless @resource.active_for_authentication?
 
     I18n.with_locale(locale) do
-      mail to: @resource.email, subject: I18n.t('devise.mailer.two_factor_recovery_codes_changed.subject')
+      mail subject: default_devise_subject
     end
   end
 
   def webauthn_enabled(user, *, **)
     @resource = user
-    @instance = Rails.configuration.x.local_domain
 
     return unless @resource.active_for_authentication?
 
     I18n.with_locale(locale) do
-      mail to: @resource.email, subject: I18n.t('devise.mailer.webauthn_enabled.subject')
+      mail subject: default_devise_subject
     end
   end
 
   def webauthn_disabled(user, *, **)
     @resource = user
-    @instance = Rails.configuration.x.local_domain
 
     return unless @resource.active_for_authentication?
 
     I18n.with_locale(locale) do
-      mail to: @resource.email, subject: I18n.t('devise.mailer.webauthn_disabled.subject')
+      mail subject: default_devise_subject
     end
   end
 
   def webauthn_credential_added(user, webauthn_credential)
     @resource = user
-    @instance = Rails.configuration.x.local_domain
     @webauthn_credential = webauthn_credential
 
     return unless @resource.active_for_authentication?
 
     I18n.with_locale(locale) do
-      mail to: @resource.email, subject: I18n.t('devise.mailer.webauthn_credential.added.subject')
+      mail subject: I18n.t('devise.mailer.webauthn_credential.added.subject')
     end
   end
 
   def webauthn_credential_deleted(user, webauthn_credential)
     @resource = user
-    @instance = Rails.configuration.x.local_domain
     @webauthn_credential = webauthn_credential
 
     return unless @resource.active_for_authentication?
 
     I18n.with_locale(locale) do
-      mail to: @resource.email, subject: I18n.t('devise.mailer.webauthn_credential.deleted.subject')
+      mail subject: I18n.t('devise.mailer.webauthn_credential.deleted.subject')
     end
   end
 
   def welcome(user)
     @resource = user
-    @instance = Rails.configuration.x.local_domain
 
     return unless @resource.active_for_authentication?
 
     I18n.with_locale(locale) do
-      mail to: @resource.email, subject: I18n.t('user_mailer.welcome.subject')
+      mail subject: default_i18n_subject
     end
   end
 
   def backup_ready(user, backup)
     @resource = user
-    @instance = Rails.configuration.x.local_domain
     @backup   = backup
 
     return unless @resource.active_for_authentication?
 
     I18n.with_locale(locale) do
-      mail to: @resource.email, subject: I18n.t('user_mailer.backup_ready.subject')
+      mail subject: default_i18n_subject
     end
   end
 
   def warning(user, warning)
     @resource = user
     @warning  = warning
-    @instance = Rails.configuration.x.local_domain
     @statuses = @warning.statuses.includes(:account, :preloadable_poll, :media_attachments, active_mentions: [:account])
 
     I18n.with_locale(locale) do
-      mail to: @resource.email, subject: I18n.t("user_mailer.warning.subject.#{@warning.action}", acct: "@#{user.account.local_username_and_domain}")
+      mail subject: I18n.t("user_mailer.warning.subject.#{@warning.action}", acct: "@#{user.account.local_username_and_domain}")
     end
   end
 
   def appeal_approved(user, appeal)
     @resource = user
-    @instance = Rails.configuration.x.local_domain
     @appeal   = appeal
 
     I18n.with_locale(locale) do
-      mail to: @resource.email, subject: I18n.t('user_mailer.appeal_approved.subject', date: l(@appeal.created_at))
+      mail subject: default_i18n_subject(date: l(@appeal.created_at))
     end
   end
 
   def appeal_rejected(user, appeal)
     @resource = user
-    @instance = Rails.configuration.x.local_domain
     @appeal   = appeal
 
     I18n.with_locale(locale) do
-      mail to: @resource.email, subject: I18n.t('user_mailer.appeal_rejected.subject', date: l(@appeal.created_at))
+      mail subject: default_i18n_subject(date: l(@appeal.created_at))
     end
   end
 
   def suspicious_sign_in(user, remote_ip, user_agent, timestamp)
     @resource   = user
-    @instance   = Rails.configuration.x.local_domain
     @remote_ip  = remote_ip
     @user_agent = user_agent
     @detection  = Browser.new(user_agent)
     @timestamp  = timestamp.to_time.utc
 
     I18n.with_locale(locale) do
-      mail to: @resource.email, subject: I18n.t('user_mailer.suspicious_sign_in.subject')
+      mail subject: default_i18n_subject
     end
   end
 
   private
 
+  def default_devise_subject
+    I18n.t(:subject, scope: ['devise.mailer', action_name])
+  end
+
+  def set_instance
+    @instance = Rails.configuration.x.local_domain
+  end
+
   def locale
     @resource.locale.presence || I18n.default_locale
   end

config/i18n-tasks.yml

@@ -64,6 +64,7 @@ ignore_unused:
   - 'statuses.attached.*'
   - 'move_handler.carry_{mutes,blocks}_over_text'
   - 'admin_mailer.*.subject'
+  - 'user_mailer.*.subject'
   - 'notification_mailer.*'
   - 'imports.overwrite_preambles.{following,blocking,muting,domain_blocking,bookmarks,lists}_html'
   - 'imports.preambles.{following,blocking,muting,domain_blocking,bookmarks,lists}_html'

spec/mailers/user_mailer_spec.rb

@@ -5,7 +5,7 @@ require 'rails_helper'
 describe UserMailer do
   let(:receiver) { Fabricate(:user) }
 
-  describe 'confirmation_instructions' do
+  describe '#confirmation_instructions' do
     let(:mail) { described_class.confirmation_instructions(receiver, 'spec') }
 
     it 'renders confirmation instructions' do
@@ -20,7 +20,7 @@ describe UserMailer do
                      instance: Rails.configuration.x.local_domain
   end
 
-  describe 'reconfirmation_instructions' do
+  describe '#reconfirmation_instructions' do
     let(:mail) { described_class.confirmation_instructions(receiver, 'spec') }
 
     it 'renders reconfirmation instructions' do
@@ -34,7 +34,7 @@ describe UserMailer do
     end
   end
 
-  describe 'reset_password_instructions' do
+  describe '#reset_password_instructions' do
     let(:mail) { described_class.reset_password_instructions(receiver, 'spec') }
 
     it 'renders reset password instructions' do
@@ -47,7 +47,7 @@ describe UserMailer do
                      'devise.mailer.reset_password_instructions.subject'
   end
 
-  describe 'password_change' do
+  describe '#password_change' do
     let(:mail) { described_class.password_change(receiver) }
 
     it 'renders password change notification' do
@@ -59,7 +59,7 @@ describe UserMailer do
                      'devise.mailer.password_change.subject'
   end
 
-  describe 'email_changed' do
+  describe '#email_changed' do
     let(:mail) { described_class.email_changed(receiver) }
 
     it 'renders email change notification' do
@@ -71,7 +71,7 @@ describe UserMailer do
                      'devise.mailer.email_changed.subject'
   end
 
-  describe 'warning' do
+  describe '#warning' do
     let(:strike) { Fabricate(:account_warning, target_account: receiver.account, text: 'dont worry its just the testsuite', action: 'suspend') }
     let(:mail)   { described_class.warning(receiver, strike) }
 
@@ -82,7 +82,7 @@ describe UserMailer do
     end
   end
 
-  describe 'webauthn_credential_deleted' do
+  describe '#webauthn_credential_deleted' do
     let(:credential) { Fabricate(:webauthn_credential, user_id: receiver.id) }
     let(:mail) { described_class.webauthn_credential_deleted(receiver, credential) }
 
@@ -95,7 +95,7 @@ describe UserMailer do
                      'devise.mailer.webauthn_credential.deleted.subject'
   end
 
-  describe 'suspicious_sign_in' do
+  describe '#suspicious_sign_in' do
     let(:ip) { '192.168.0.1' }
     let(:agent) { 'NCSA_Mosaic/2.0 (Windows 3.1)' }
     let(:timestamp) { Time.now.utc }
@@ -110,7 +110,7 @@ describe UserMailer do
                      'user_mailer.suspicious_sign_in.subject'
   end
 
-  describe 'appeal_approved' do
+  describe '#appeal_approved' do
     let(:appeal) { Fabricate(:appeal, account: receiver.account, approved_at: Time.now.utc) }
     let(:mail) { described_class.appeal_approved(receiver, appeal) }
 
@@ -120,7 +120,7 @@ describe UserMailer do
     end
   end
 
-  describe 'appeal_rejected' do
+  describe '#appeal_rejected' do
     let(:appeal) { Fabricate(:appeal, account: receiver.account, rejected_at: Time.now.utc) }
     let(:mail) { described_class.appeal_rejected(receiver, appeal) }
 
@@ -130,7 +130,7 @@ describe UserMailer do
     end
   end
 
-  describe 'two_factor_enabled' do
+  describe '#two_factor_enabled' do
     let(:mail) { described_class.two_factor_enabled(receiver) }
 
     it 'renders two_factor_enabled mail' do
@@ -139,7 +139,7 @@ describe UserMailer do
     end
   end
 
-  describe 'two_factor_disabled' do
+  describe '#two_factor_disabled' do
     let(:mail) { described_class.two_factor_disabled(receiver) }
 
     it 'renders two_factor_disabled mail' do
@@ -148,7 +148,7 @@ describe UserMailer do
     end
   end
 
-  describe 'webauthn_enabled' do
+  describe '#webauthn_enabled' do
     let(:mail) { described_class.webauthn_enabled(receiver) }
 
     it 'renders webauthn_enabled mail' do
@@ -157,7 +157,7 @@ describe UserMailer do
     end
   end
 
-  describe 'webauthn_disabled' do
+  describe '#webauthn_disabled' do
     let(:mail) { described_class.webauthn_disabled(receiver) }
 
     it 'renders webauthn_disabled mail' do
@@ -166,7 +166,7 @@ describe UserMailer do
     end
   end
 
-  describe 'two_factor_recovery_codes_changed' do
+  describe '#two_factor_recovery_codes_changed' do
     let(:mail) { described_class.two_factor_recovery_codes_changed(receiver) }
 
     it 'renders two_factor_recovery_codes_changed mail' do
@@ -175,7 +175,7 @@ describe UserMailer do
     end
   end
 
-  describe 'webauthn_credential_added' do
+  describe '#webauthn_credential_added' do
     let(:credential) { Fabricate.build(:webauthn_credential) }
     let(:mail) { described_class.webauthn_credential_added(receiver, credential) }
 
@@ -184,4 +184,23 @@ describe UserMailer do
       expect(mail.body.encoded).to include I18n.t('devise.mailer.webauthn_credential.added.explanation')
     end
   end
+
+  describe '#welcome' do
+    let(:mail) { described_class.welcome(receiver) }
+
+    it 'renders welcome mail' do
+      expect(mail.subject).to eq I18n.t('user_mailer.welcome.subject')
+      expect(mail.body.encoded).to include I18n.t('user_mailer.welcome.explanation')
+    end
+  end
+
+  describe '#backup_ready' do
+    let(:backup) { Fabricate(:backup) }
+    let(:mail) { described_class.backup_ready(receiver, backup) }
+
+    it 'renders backup_ready mail' do
+      expect(mail.subject).to eq I18n.t('user_mailer.backup_ready.subject')
+      expect(mail.body.encoded).to include I18n.t('user_mailer.backup_ready.explanation')
+    end
+  end
 end