From bd1b4d31302dbaef5f1a210038deff749f983017 Mon Sep 17 00:00:00 2001
From: Robin Ward <robin.ward@gmail.com>
Date: Thu, 13 Jun 2013 17:00:00 -0400
Subject: [PATCH] Include a custom reply address when reply by email is
 enabled.

---
 ...612200846_create_post_upload_join_table.rb |  2 +-
 lib/email/message_builder.rb                  | 63 ++++++++++++++-----
 spec/components/email/message_builder_spec.rb | 20 +++++-
 3 files changed, 65 insertions(+), 20 deletions(-)

diff --git a/db/migrate/20130612200846_create_post_upload_join_table.rb b/db/migrate/20130612200846_create_post_upload_join_table.rb
index 5af4fe5b3..94698c89f 100644
--- a/db/migrate/20130612200846_create_post_upload_join_table.rb
+++ b/db/migrate/20130612200846_create_post_upload_join_table.rb
@@ -1,6 +1,6 @@
 class CreatePostUploadJoinTable < ActiveRecord::Migration
   def change
-  	create_table :posts_uploads, id: false do |t|
+  	create_table :posts_uploads, force: true, id: false do |t|
   		t.integer :post_id
   		t.integer :upload_id
   	end
diff --git a/lib/email/message_builder.rb b/lib/email/message_builder.rb
index 7aed31e81..a97e55535 100644
--- a/lib/email/message_builder.rb
+++ b/lib/email/message_builder.rb
@@ -35,10 +35,6 @@ module Email
       body
     end
 
-    def allow_reply_by_email?
-      SiteSetting.reply_by_email_enabled? && @opts[:allow_reply_by_email]
-    end
-
     def template_args
       @template_args ||= { site_name: SiteSetting.title,
                            base_url: Discourse.base_url,
@@ -46,17 +42,11 @@ module Email
     end
 
     def build_args
-      mail_args = { to: @to,
-                    subject: subject,
-                    body: body,
-                    charset: 'UTF-8' }
-
-      mail_args[:from] = @opts[:from] || SiteSetting.notification_email
-
-      if @opts[:from_alias]
-        mail_args[:from] = "#{@opts[:from_alias]} <#{mail_args[:from]}>"
-      end
-      mail_args
+      { to: @to,
+        subject: subject,
+        body: body,
+        charset: 'UTF-8',
+        from: from_value }
     end
 
     def header_args
@@ -65,11 +55,52 @@ module Email
         result['List-Unsubscribe'] = "<#{template_args[:user_preferences_url]}>" if @opts[:add_unsubscribe_link]
       end
 
-      result['Discourse-Reply-Key'] = SecureRandom.hex(16) if allow_reply_by_email?
+      if allow_reply_by_email?
+        result['Discourse-Reply-Key'] = reply_key
+        result['Reply-To'] = reply_by_email_address
+      else
+        result['Reply-To'] = from_value
+      end
 
       result
     end
 
+
+    protected
+
+    def reply_key
+      @reply_key ||= SecureRandom.hex(16)
+    end
+
+    def allow_reply_by_email?
+      SiteSetting.reply_by_email_enabled? &&
+      reply_by_email_address.present? &&
+      @opts[:allow_reply_by_email]
+    end
+
+    def from_value
+      return @from_value if @from_value
+      @from_value = @opts[:from] || SiteSetting.notification_email
+      @from_value = alias_email(@from_value)
+      @from_value
+    end
+
+    def reply_by_email_address
+      return @reply_by_email_address if @reply_by_email_address
+      return nil unless SiteSetting.reply_by_email_address.present?
+
+      @reply_by_email_address = SiteSetting.reply_by_email_address.dup
+      @reply_by_email_address.gsub!("%{reply_key}", reply_key)
+      @reply_by_email_address = alias_email(@reply_by_email_address)
+
+      @reply_by_email_address
+    end
+
+    def alias_email(source)
+      return source if @opts[:from_alias].blank?
+      "#{@opts[:from_alias]} <#{source}>"
+    end
+
   end
 
 end
diff --git a/spec/components/email/message_builder_spec.rb b/spec/components/email/message_builder_spec.rb
index a392fcb7a..dd83c393a 100644
--- a/spec/components/email/message_builder_spec.rb
+++ b/spec/components/email/message_builder_spec.rb
@@ -8,6 +8,7 @@ describe Email::MessageBuilder do
   let(:body) { "oh my glob Jake, Tree Trunks just made the tastiest apple pie ever!"}
   let(:builder) { Email::MessageBuilder.new(to_address, subject: subject, body: body) }
   let(:build_args) { builder.build_args }
+  let(:header_args) { builder.header_args }
 
   it "has the correct to address" do
     expect(build_args[:to]).to eq(to_address)
@@ -29,7 +30,11 @@ describe Email::MessageBuilder do
 
     context "without allow_reply_by_email" do
       it "does not have a Discourse-Reply-Key" do
-        expect(builder.header_args['Discourse-Reply-Key']).to be_blank
+        expect(header_args['Discourse-Reply-Key']).to be_blank
+      end
+
+      it "returns a Reply-To header that's the same as From" do
+        expect(header_args['Reply-To']).to eq(build_args[:from])
       end
     end
 
@@ -39,23 +44,32 @@ describe Email::MessageBuilder do
 
       context "With the SiteSetting enabled" do
         before do
-          SiteSetting.expects(:reply_by_email_enabled?).returns(true)
+          SiteSetting.stubs(:reply_by_email_enabled?).returns(true)
+          SiteSetting.stubs(:reply_by_email_address).returns("r+%{reply_key}@reply.myforum.com")
         end
 
         it "has a Discourse-Reply-Key" do
           expect(reply_key).to be_present
           expect(reply_key.size).to eq(32)
         end
+
+        it "returns a Reply-To header with the reply key" do
+          expect(reply_by_email_builder.header_args['Reply-To']).to eq("r+#{reply_key}@reply.myforum.com")
+        end
       end
 
       context "With the SiteSetting disabled" do
         before do
-          SiteSetting.expects(:reply_by_email_enabled?).returns(false)
+          SiteSetting.stubs(:reply_by_email_enabled?).returns(false)
         end
 
         it "has no Discourse-Reply-Key" do
           expect(reply_key).to be_blank
         end
+
+        it "returns a Reply-To header that's the same as From" do
+          expect(header_args['Reply-To']).to eq(build_args[:from])
+        end
       end
     end