Use before_action to handle missing status scenario in NotificationMailer

This commit is contained in:
Matt Jankowski 2024-09-25 10:59:00 -04:00
parent 51777fe3e2
commit d3a1c2be6b
3 changed files with 21 additions and 6 deletions

View File

@ -9,6 +9,8 @@ class ApplicationMailer < ActionMailer::Base
after_action :set_autoreply_headers! after_action :set_autoreply_headers!
rescue_from UncaughtThrowError, with: -> {}
protected protected
def locale_for_account(account, &block) def locale_for_account(account, &block)

View File

@ -11,6 +11,7 @@ class NotificationMailer < ApplicationMailer
after_action :thread_by_conversation! after_action :thread_by_conversation!
end end
before_action :set_account, only: [:follow, :favourite, :reblog, :follow_request] before_action :set_account, only: [:follow, :favourite, :reblog, :follow_request]
before_action :verify_status_presence, only: %i(favourite mention reblog)
after_action :set_list_headers! after_action :set_list_headers!
before_deliver :verify_functional_user before_deliver :verify_functional_user
@ -20,8 +21,6 @@ class NotificationMailer < ApplicationMailer
layout 'mailer' layout 'mailer'
def mention def mention
return if @status.blank?
locale_for_account(@me) do locale_for_account(@me) do
mail subject: default_i18n_subject(name: @status.account.acct) mail subject: default_i18n_subject(name: @status.account.acct)
end end
@ -34,16 +33,12 @@ class NotificationMailer < ApplicationMailer
end end
def favourite def favourite
return if @status.blank?
locale_for_account(@me) do locale_for_account(@me) do
mail subject: default_i18n_subject(name: @account.acct) mail subject: default_i18n_subject(name: @account.acct)
end end
end end
def reblog def reblog
return if @status.blank?
locale_for_account(@me) do locale_for_account(@me) do
mail subject: default_i18n_subject(name: @account.acct) mail subject: default_i18n_subject(name: @account.acct)
end end
@ -77,6 +72,10 @@ class NotificationMailer < ApplicationMailer
throw(:abort) unless @user.functional? throw(:abort) unless @user.functional?
end end
def verify_status_presence
throw(:abort) if @status.blank?
end
def set_list_headers! def set_list_headers!
headers( headers(
'List-ID' => "<#{@type}.#{@me.username}.#{Rails.configuration.x.local_domain}>", 'List-ID' => "<#{@type}.#{@me.username}.#{Rails.configuration.x.local_domain}>",

View File

@ -14,6 +14,17 @@ RSpec.describe NotificationMailer do
end end
end end
shared_examples 'delivery without status' do
context 'when notification target_status is missing' do
before { allow(notification).to receive(:target_status).and_return(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(:receiver) { Fabricate(:user, account_attributes: { username: 'alice' }) }
let(:sender) { Fabricate(:account, username: 'bob') } let(:sender) { Fabricate(:account, username: 'bob') }
let(:foreign_status) { Fabricate(:status, account: sender, text: 'The body of the foreign status') } let(:foreign_status) { Fabricate(:status, account: sender, text: 'The body of the foreign status') }
@ -37,6 +48,7 @@ RSpec.describe NotificationMailer do
end end
include_examples 'delivery to non functional user' include_examples 'delivery to non functional user'
include_examples 'delivery without status'
end end
describe 'follow' do describe 'follow' do
@ -75,6 +87,7 @@ RSpec.describe NotificationMailer do
end end
include_examples 'delivery to non functional user' include_examples 'delivery to non functional user'
include_examples 'delivery without status'
end end
describe 'reblog' do describe 'reblog' do
@ -95,6 +108,7 @@ RSpec.describe NotificationMailer do
end end
include_examples 'delivery to non functional user' include_examples 'delivery to non functional user'
include_examples 'delivery without status'
end end
describe 'follow_request' do describe 'follow_request' do