FIX: Add topic deleted check to email/sender (#9166)

It already had a deleted post check and log reason, add a topic one too to avoid errors
This commit is contained in:
Martin Brennan 2020-03-13 10:04:15 +10:00 committed by GitHub
parent 4d7f6749be
commit 2237ba8c9d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 27 additions and 5 deletions

View File

@ -36,7 +36,8 @@ class SkippedEmailLog < ActiveRecord::Base
sender_body_blank: 19,
sender_post_deleted: 20,
sender_message_to_invalid: 21,
user_email_access_denied: 22
user_email_access_denied: 22,
sender_topic_deleted: 23
# you need to add the reason in server.en.yml below the "skipped_email_log" key
# when you add a new enum value
)

View File

@ -3771,6 +3771,7 @@ en:
sender_body_blank: "body is blank"
sender_post_deleted: "post has been deleted"
sender_message_to_invalid: "recipient has invalid email address"
sender_topic_deleted: "topic has been deleted"
color_schemes:
base_theme_name: "Base"

View File

@ -96,12 +96,13 @@ module Email
if topic_id.present? && post_id.present?
post = Post.find_by(id: post_id, topic_id: topic_id)
# guards against deleted posts
return skip(SkippedEmailLog.reason_types[:sender_post_deleted]) unless post
add_attachments(post)
# guards against deleted posts and topics
return skip(SkippedEmailLog.reason_types[:sender_post_deleted]) if post.blank?
topic = post.topic
return skip(SkippedEmailLog.reason_types[:sender_topic_deleted]) if topic.blank?
add_attachments(post)
first_post = topic.ordered_posts.first
topic_message_id = first_post.incoming_email&.message_id.present? ?

View File

@ -443,6 +443,25 @@ describe Email::Sender do
end
context 'with a deleted topic' do
it 'should skip sending the email' do
post = Fabricate(:post, topic: Fabricate(:topic, deleted_at: 1.day.ago))
message = Mail::Message.new to: 'disc@ourse.org', body: 'some content'
message.header['X-Discourse-Post-Id'] = post.id
message.header['X-Discourse-Topic-Id'] = post.topic_id
message.expects(:deliver_now).never
email_sender = Email::Sender.new(message, :valid_type)
expect { email_sender.send }.to change { SkippedEmailLog.count }
log = SkippedEmailLog.last
expect(log.reason_type).to eq(SkippedEmailLog.reason_types[:sender_topic_deleted])
end
end
context 'with a user' do
let(:message) do
message = Mail::Message.new to: 'eviltrout@test.domain', body: 'test body'