FIX: create whisper post in PMs when bounces with verp and user is staged

FIX: create whisper post in PMs when bounces with verp and user is staged

From 7dbf709467c85b3cf4cfc69dbd9150aeef77f850 Mon Sep 17 00:00:00 2001
From: Vinoth Kannan <vinothkannan@vinkas.com>
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


diff --git a/lib/email/receiver.rb b/lib/email/receiver.rb
index c17b238..0a9e4ef 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 4b64d87..c178dd6 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

GitHub

1 Like