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