From 2237ba8c9d235a5e6b34c548253a34630fb042fc Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Fri, 13 Mar 2020 10:04:15 +1000 Subject: [PATCH] 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 --- app/models/skipped_email_log.rb | 3 ++- config/locales/server.en.yml | 1 + lib/email/sender.rb | 9 +++++---- spec/components/email/sender_spec.rb | 19 +++++++++++++++++++ 4 files changed, 27 insertions(+), 5 deletions(-) diff --git a/app/models/skipped_email_log.rb b/app/models/skipped_email_log.rb index 374e8cff0d8..9ffa2230e97 100644 --- a/app/models/skipped_email_log.rb +++ b/app/models/skipped_email_log.rb @@ -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 ) diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 4b7298af425..ba7ead37b0b 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -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" diff --git a/lib/email/sender.rb b/lib/email/sender.rb index bce5fc90de1..8cd3c613fd2 100644 --- a/lib/email/sender.rb +++ b/lib/email/sender.rb @@ -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? ? diff --git a/spec/components/email/sender_spec.rb b/spec/components/email/sender_spec.rb index a93e1cd410b..26d4dcffbe9 100644 --- a/spec/components/email/sender_spec.rb +++ b/spec/components/email/sender_spec.rb @@ -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'