From a03287f2ee2a2f2d7ae363cad9ebc04c6586318c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9gis=20Hanol?= <regis@hanol.fr>
Date: Mon, 28 Nov 2016 14:18:02 +0100
Subject: [PATCH] FIX: 'In-Reply-To' header should default to topic_message_id

---
 lib/email/sender.rb                  |  4 +-
 spec/components/email/sender_spec.rb | 70 +++++++++++++++++++++++++++-
 2 files changed, 71 insertions(+), 3 deletions(-)

diff --git a/lib/email/sender.rb b/lib/email/sender.rb
index ea8d982c77e..e5bee8c03ac 100644
--- a/lib/email/sender.rb
+++ b/lib/email/sender.rb
@@ -100,8 +100,8 @@ module Email
 
         @message.header['Message-ID'] = incoming_message_id || post_message_id
         if post && post.post_number > 1
-          @message.header['In-Reply-To'] = referenced_post_message_ids.first
-          @message.header['References'] = [topic_message_id, referenced_post_message_ids].flatten
+          @message.header['In-Reply-To'] = referenced_post_message_ids.first || topic_message_id
+          @message.header['References'] = [topic_message_id, referenced_post_message_ids].flatten.compact.uniq
         end
 
         # http://www.ietf.org/rfc/rfc2919.txt
diff --git a/spec/components/email/sender_spec.rb b/spec/components/email/sender_spec.rb
index 1fdfe23874a..b3d53ccce4b 100644
--- a/spec/components/email/sender_spec.rb
+++ b/spec/components/email/sender_spec.rb
@@ -139,6 +139,75 @@ describe Email::Sender do
       Then { expect(message.header['X-Discourse-Reply-Key']).not_to be_present }
     end
 
+    context "email threading" do
+      let(:topic) { Fabricate(:topic) }
+
+      let(:post_1) { Fabricate(:post, topic: topic, post_number: 1) }
+      let(:post_2) { Fabricate(:post, topic: topic, post_number: 2) }
+      let(:post_3) { Fabricate(:post, topic: topic, post_number: 3) }
+      let(:post_4) { Fabricate(:post, topic: topic, post_number: 4) }
+
+      let!(:incoming_email) { IncomingEmail.create(topic: topic, post: post_4, message_id: "foobar") }
+
+      let!(:post_reply_1_3) { PostReply.create(post: post_1, reply: post_3) }
+      let!(:post_reply_2_3) { PostReply.create(post: post_2, reply: post_3) }
+
+      before do
+        message.header['X-Discourse-Topic-Id'] = topic.id
+      end
+
+      it "doesn't set the 'In-Reply-To' and 'References' headers on the first post" do
+        message.header['X-Discourse-Post-Id'] = post_1.id
+
+        email_sender.send
+
+        expect(message.header['Message-Id'].to_s).to eq("<topic/#{topic.id}/#{post_1.id}@test.localhost>")
+        expect(message.header['In-Reply-To'].to_s).to be_blank
+        expect(message.header['References'].to_s).to be_blank
+      end
+
+      it "sets the 'In-Reply-To' header to the topic by default" do
+        message.header['X-Discourse-Post-Id'] = post_2.id
+
+        email_sender.send
+
+        expect(message.header['Message-Id'].to_s).to eq("<topic/#{topic.id}/#{post_2.id}@test.localhost>")
+        expect(message.header['In-Reply-To'].to_s).to eq("<topic/#{topic.id}@test.localhost>")
+      end
+
+      it "sets the 'In-Reply-To' header to the newest replied post" do
+        message.header['X-Discourse-Post-Id'] = post_3.id
+
+        email_sender.send
+
+        expect(message.header['Message-Id'].to_s).to eq("<topic/#{topic.id}/#{post_3.id}@test.localhost>")
+        expect(message.header['In-Reply-To'].to_s).to eq("<topic/#{topic.id}/#{post_2.id}@test.localhost>")
+      end
+
+      it "sets the 'References' header to the topic and all replied posts" do
+        message.header['X-Discourse-Post-Id'] = post_3.id
+
+        email_sender.send
+
+        references = [
+          "<topic/#{topic.id}@test.localhost>",
+          "<topic/#{topic.id}/#{post_2.id}@test.localhost>",
+          "<topic/#{topic.id}/#{post_1.id}@test.localhost>",
+        ]
+
+        expect(message.header['References'].to_s).to eq(references.join(" "))
+      end
+
+      it "uses the incoming_email message_id when available" do
+        message.header['X-Discourse-Post-Id'] = post_4.id
+
+        email_sender.send
+
+        expect(message.header['Message-Id'].to_s).to eq("<#{incoming_email.message_id}>")
+      end
+
+    end
+
     context "merges custom mandrill header" do
       before do
         ActionMailer::Base.smtp_settings[:address] = "smtp.mandrillapp.com"
@@ -221,7 +290,6 @@ describe Email::Sender do
       expect(@email_log.user_id).to eq(user.id)
     end
 
-
   end
 
 end