From 0af1242aa8fb96cb26961715cba6161c62f058e4 Mon Sep 17 00:00:00 2001 From: Peter N Lewis Date: Fri, 6 Jun 2014 21:09:00 +0800 Subject: [PATCH 1/2] Use an appropriate name in the Reply-To header Use "Site Name <>" for the Reply-To header when the reply is to the site or a public topic. Use "username <>" for the Reply-To header only when the reply is to a private message topic. --- app/mailers/user_notifications.rb | 1 + lib/email/message_builder.rb | 17 ++++++++++++++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/app/mailers/user_notifications.rb b/app/mailers/user_notifications.rb index 5981e879699..f86bc2cdc41 100644 --- a/app/mailers/user_notifications.rb +++ b/app/mailers/user_notifications.rb @@ -215,6 +215,7 @@ class UserNotifications < ActionMailer::Base username: from_alias, add_unsubscribe_link: true, allow_reply_by_email: allow_reply_by_email, + private_reply: post.topic.private_message?, include_respond_instructions: !user.suspended?, template: template, html_override: html, diff --git a/lib/email/message_builder.rb b/lib/email/message_builder.rb index 130d3a7cf96..bba5be187d7 100644 --- a/lib/email/message_builder.rb +++ b/lib/email/message_builder.rb @@ -136,6 +136,13 @@ module Email @opts[:allow_reply_by_email] end + def private_reply? + SiteSetting.reply_by_email_enabled? && + reply_by_email_address.present? && + @opts[:allow_reply_by_email] && + @opts[:private_reply] + end + def from_value return @from_value if @from_value @from_value = @opts[:from] || SiteSetting.notification_email @@ -149,7 +156,11 @@ module Email @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 = if private_reply? + alias_email(@reply_by_email_address) + else + site_alias_email(@reply_by_email_address) + end @reply_by_email_address end @@ -159,6 +170,10 @@ module Email "#{@opts[:from_alias]} <#{source}>" end + def site_alias_email(source) + return "#{SiteSetting.title} <#{source}>" + end + end end From 93f5f98b580f3875a10c9da948bd9ff23bd92f2b Mon Sep 17 00:00:00 2001 From: Peter N Lewis Date: Mon, 9 Jun 2014 18:26:19 +0800 Subject: [PATCH 2/2] Corrected and added appropriate specs to confirm correct behaviour. Tests ensure that the site name is used for public replies and the username is used for private replies. --- spec/components/email/message_builder_spec.rb | 37 ++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/spec/components/email/message_builder_spec.rb b/spec/components/email/message_builder_spec.rb index 82e8961d6b5..187febf36be 100644 --- a/spec/components/email/message_builder_spec.rb +++ b/spec/components/email/message_builder_spec.rb @@ -54,7 +54,42 @@ describe Email::MessageBuilder do 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") + expect(reply_by_email_builder.header_args['Reply-To']).to eq(SiteSetting.title + " ") + end + end + + context "With the SiteSetting disabled" do + before do + SiteSetting.stubs(:reply_by_email_enabled?).returns(false) + end + + it "has no X-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 + + context "with allow_reply_by_email" do + let(:reply_by_email_builder) { Email::MessageBuilder.new(to_address, allow_reply_by_email: true, private_reply: true, from_alias: "Username") } + let(:reply_key) { reply_by_email_builder.header_args['X-Discourse-Reply-Key'] } + + context "With the SiteSetting enabled" do + before do + 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 X-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("Username ") end end