From 7dbf709467c85b3cf4cfc69dbd9150aeef77f850 Mon Sep 17 00:00:00 2001 From: Vinoth Kannan Date: Wed, 28 Nov 2018 08:24:23 +0530 Subject: [PATCH] FIX: create whisper post in PMs when bounces with verp and user is staged --- lib/email/receiver.rb | 25 +++++++----- spec/components/email/receiver_spec.rb | 55 +++++++++++++++----------- 2 files changed, 49 insertions(+), 31 deletions(-) diff --git a/lib/email/receiver.rb b/lib/email/receiver.rb index c17b238d644..0a9e4effd89 100644 --- a/lib/email/receiver.rb +++ b/lib/email/receiver.rb @@ -196,13 +196,13 @@ module Email end def is_bounce? - @mail.bounced? || verp + @mail.bounced? || bounce_key end def handle_bounce @incoming_email.update_columns(is_bounce: true) - if verp && (bounce_key = verp[/\+verp-(\h{32})@/, 1]) && (email_log = EmailLog.find_by(bounce_key: bounce_key)) + if bounce_key && (email_log = EmailLog.find_by(bounce_key: bounce_key)) email_log.update_columns(bounced: true) user = email_log.user email = user&.email.presence @@ -229,8 +229,11 @@ module Email Email::Receiver.reply_by_email_address_regex.match(@from_email) end - def verp - @verp ||= all_destinations.select { |to| to[/\+verp-\h{32}@/] }.first + def bounce_key + @bounce_key ||= begin + verp = all_destinations.select { |to| to[/\+verp-\h{32}@/] }.first + verp && verp[/\+verp-(\h{32})@/, 1] + end end def self.update_bounce_score(email, score) @@ -555,7 +558,7 @@ module Email def destinations @destinations ||= all_destinations - .map { |d| Email::Receiver.check_address(d) } + .map { |d| Email::Receiver.check_address(d, is_bounce?) } .reject(&:blank?) end @@ -572,7 +575,7 @@ module Email end end - def self.check_address(address) + def self.check_address(address, include_verp = false) # only check for a group/category when 'email_in' is enabled if SiteSetting.email_in group = Group.find_by_email(address) @@ -583,7 +586,7 @@ module Email end # reply - match = Email::Receiver.reply_by_email_address_regex.match(address) + match = Email::Receiver.reply_by_email_address_regex(true, include_verp).match(address) if match && match.captures match.captures.each do |c| next if c.blank? @@ -785,10 +788,14 @@ module Email true end - def self.reply_by_email_address_regex(extract_reply_key = true) + def self.reply_by_email_address_regex(extract_reply_key = true, include_verp = false) reply_addresses = [SiteSetting.reply_by_email_address] reply_addresses << (SiteSetting.alternative_reply_by_email_addresses.presence || "").split("|") + if include_verp && SiteSetting.reply_by_email_address.present? && SiteSetting.reply_by_email_address["+"] + reply_addresses << SiteSetting.reply_by_email_address.sub("%{reply_key}", "verp-%{reply_key}") + end + reply_addresses.flatten! reply_addresses.select!(&:present?) reply_addresses.map! { |a| Regexp.escape(a) } @@ -1028,7 +1035,7 @@ module Email if result.post @incoming_email.update_columns(topic_id: result.post.topic_id, post_id: result.post.id) - if result.post.topic && result.post.topic.private_message? + if result.post.topic&.private_message? && !is_bounce? add_other_addresses(result.post, user) end end diff --git a/spec/components/email/receiver_spec.rb b/spec/components/email/receiver_spec.rb index 4b64d8711ed..c178dd6ff54 100644 --- a/spec/components/email/receiver_spec.rb +++ b/spec/components/email/receiver_spec.rb @@ -112,33 +112,44 @@ describe Email::Receiver do expect(IncomingEmail.last.is_bounce).to eq(true) end - it "creates a whisper post in PM if user is staged" do - SiteSetting.enable_staged_users = true - SiteSetting.enable_whispers = true + describe "creating whisper post in PMs for staged users" do + let(:email_address) { "linux-admin@b-s-c.co.jp" } - email = "linux-admin@b-s-c.co.jp" - user = Fabricate(:staged, email: email) + before do + SiteSetting.enable_staged_users = true + SiteSetting.enable_whispers = true + end - private_message = Fabricate(:topic, - archetype: 'private_message', - category_id: nil, - user: user, - allowed_users: [user] - ) + def create_post_reply_key(value) + user = Fabricate(:staged, email: email_address) + pm = Fabricate(:topic, archetype: 'private_message', category_id: nil, user: user, allowed_users: [user]) + Fabricate(:post_reply_key, + reply_key: value, + user: user, + post: create_post(topic: pm, user: user) + ) + end - post = create_post(topic: private_message, user: user) + it "when bounce without verp" do + create_post_reply_key("4f97315cc828096c9cb34c6f1a0d6fe8") - post_reply_key = Fabricate(:post_reply_key, - reply_key: "4f97315cc828096c9cb34c6f1a0d6fe8", - user: user, - post: post - ) + expect { process(:bounced_email) }.to raise_error(Email::Receiver::BouncedEmailError) + post = Post.last + expect(post.whisper?).to eq(true) + expect(post.raw).to eq(I18n.t("system_messages.email_bounced", email: email_address, raw: "Your email bounced").strip) + expect(IncomingEmail.last.is_bounce).to eq(true) + end - expect { process(:bounced_email) }.to raise_error(Email::Receiver::BouncedEmailError) - post = Post.last - expect(post.whisper?).to eq(true) - expect(post.raw).to eq(I18n.t("system_messages.email_bounced", email: email, raw: "Your email bounced").strip) - expect(IncomingEmail.last.is_bounce).to eq(true) + it "when bounce without verp" do + SiteSetting.reply_by_email_address = "foo+%{reply_key}@discourse.org" + create_post_reply_key("14b08c855160d67f2e0c2f8ef36e251e") + + expect { process(:hard_bounce_via_verp) }.to raise_error(Email::Receiver::BouncedEmailError) + post = Post.last + expect(post.whisper?).to eq(true) + expect(post.raw).to eq(I18n.t("system_messages.email_bounced", email: email_address, raw: "Your email bounced").strip) + expect(IncomingEmail.last.is_bounce).to eq(true) + end end end