From 83574f641aba9278bbc9c0d940717dad00522634 Mon Sep 17 00:00:00 2001
From: Matt Jankowski <matt@jankowski.online>
Date: Wed, 25 Sep 2024 04:07:48 -0400
Subject: [PATCH] Add coverage and use mailer callback to check functional user
 in notification mailer (#32055)

---
 app/mailers/notification_mailer.rb       | 16 +++++++++-------
 spec/mailers/notification_mailer_spec.rb | 21 +++++++++++++++++++++
 2 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/app/mailers/notification_mailer.rb b/app/mailers/notification_mailer.rb
index 6b21b4bed..4c374f5d5 100644
--- a/app/mailers/notification_mailer.rb
+++ b/app/mailers/notification_mailer.rb
@@ -13,12 +13,14 @@ class NotificationMailer < ApplicationMailer
   before_action :set_account, only: [:follow, :favourite, :reblog, :follow_request]
   after_action :set_list_headers!
 
+  before_deliver :verify_functional_user
+
   default to: -> { email_address_with_name(@user.email, @me.username) }
 
   layout 'mailer'
 
   def mention
-    return unless @user.functional? && @status.present?
+    return if @status.blank?
 
     locale_for_account(@me) do
       mail subject: default_i18n_subject(name: @status.account.acct)
@@ -26,15 +28,13 @@ class NotificationMailer < ApplicationMailer
   end
 
   def follow
-    return unless @user.functional?
-
     locale_for_account(@me) do
       mail subject: default_i18n_subject(name: @account.acct)
     end
   end
 
   def favourite
-    return unless @user.functional? && @status.present?
+    return if @status.blank?
 
     locale_for_account(@me) do
       mail subject: default_i18n_subject(name: @account.acct)
@@ -42,7 +42,7 @@ class NotificationMailer < ApplicationMailer
   end
 
   def reblog
-    return unless @user.functional? && @status.present?
+    return if @status.blank?
 
     locale_for_account(@me) do
       mail subject: default_i18n_subject(name: @account.acct)
@@ -50,8 +50,6 @@ class NotificationMailer < ApplicationMailer
   end
 
   def follow_request
-    return unless @user.functional?
-
     locale_for_account(@me) do
       mail subject: default_i18n_subject(name: @account.acct)
     end
@@ -75,6 +73,10 @@ class NotificationMailer < ApplicationMailer
     @account = @notification.from_account
   end
 
+  def verify_functional_user
+    throw(:abort) unless @user.functional?
+  end
+
   def set_list_headers!
     headers(
       'List-ID' => "<#{@type}.#{@me.username}.#{Rails.configuration.x.local_domain}>",
diff --git a/spec/mailers/notification_mailer_spec.rb b/spec/mailers/notification_mailer_spec.rb
index 056000b04..4c6107d9f 100644
--- a/spec/mailers/notification_mailer_spec.rb
+++ b/spec/mailers/notification_mailer_spec.rb
@@ -3,6 +3,17 @@
 require 'rails_helper'
 
 RSpec.describe NotificationMailer do
+  shared_examples 'delivery to non functional user' do
+    context 'when user is not functional' do
+      before { receiver.update(confirmed_at: nil) }
+
+      it 'does not deliver mail' do
+        emails = capture_emails { mail.deliver_now }
+        expect(emails).to be_empty
+      end
+    end
+  end
+
   let(:receiver)       { Fabricate(:user, account_attributes: { username: 'alice' }) }
   let(:sender)         { Fabricate(:account, username: 'bob') }
   let(:foreign_status) { Fabricate(:status, account: sender, text: 'The body of the foreign status') }
@@ -24,6 +35,8 @@ RSpec.describe NotificationMailer do
         .and have_thread_headers
         .and have_standard_headers('mention').for(receiver)
     end
+
+    include_examples 'delivery to non functional user'
   end
 
   describe 'follow' do
@@ -40,6 +53,8 @@ RSpec.describe NotificationMailer do
         .and(have_body_text('bob is now following you'))
         .and have_standard_headers('follow').for(receiver)
     end
+
+    include_examples 'delivery to non functional user'
   end
 
   describe 'favourite' do
@@ -58,6 +73,8 @@ RSpec.describe NotificationMailer do
         .and have_thread_headers
         .and have_standard_headers('favourite').for(receiver)
     end
+
+    include_examples 'delivery to non functional user'
   end
 
   describe 'reblog' do
@@ -76,6 +93,8 @@ RSpec.describe NotificationMailer do
         .and have_thread_headers
         .and have_standard_headers('reblog').for(receiver)
     end
+
+    include_examples 'delivery to non functional user'
   end
 
   describe 'follow_request' do
@@ -92,6 +111,8 @@ RSpec.describe NotificationMailer do
         .and(have_body_text('bob has requested to follow you'))
         .and have_standard_headers('follow_request').for(receiver)
     end
+
+    include_examples 'delivery to non functional user'
   end
 
   private