From 013ad0fdda04f24088d0990871074736e1dc60b9 Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Mon, 8 Jul 2013 11:48:40 -0400 Subject: [PATCH] Added `In-Reply-To` and `References` email headers. Additionally removed username from email replies and new posts to keep the subjects collapsable. --- config/locales/server.en.yml | 4 +-- lib/email/sender.rb | 35 +++++++++++++++-------- spec/components/email/sender_spec.rb | 42 +++++++++++++++++----------- 3 files changed, 51 insertions(+), 30 deletions(-) diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index bab9d3edfd1..b8e5fa6af95 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -934,7 +934,7 @@ en: Please visit this link to view the topic: %{base_url}%{url} user_replied: - subject_template: "[%{site_name}] %{username} replied to your post in '%{topic_title}'" + subject_template: "[%{site_name}] new reply to your post in '%{topic_title}'" text_body_template: | %{username} replied to your post in '%{topic_title}' on %{site_name}: @@ -967,7 +967,7 @@ en: %{respond_instructions} user_posted: - subject_template: "[%{site_name}] %{username} posted in '%{topic_title}'" + subject_template: "[%{site_name}] new post in '%{topic_title}'" text_body_template: | %{username} posted in '%{topic_title}' on %{site_name}: diff --git a/lib/email/sender.rb b/lib/email/sender.rb index 1311f5b0146..f770ba302c5 100644 --- a/lib/email/sender.rb +++ b/lib/email/sender.rb @@ -47,11 +47,24 @@ module Email user_id: @user.try(:id)) - @message.header['List-Id'] = Email::Sender.list_id_for(SiteSetting.title, Discourse.base_url) + host = Email::Sender.host_for(Discourse.base_url) - add_header_to_log('X-Discourse-Reply-Key', email_log, :reply_key) - add_header_to_log('X-Discourse-Post-Id', email_log, :post_id) - add_header_to_log('X-Discourse-Topic-Id', email_log, :topic_id) + @message.header['List-Id'] = Email::Sender.list_id_for(SiteSetting.title, host) + + topic_id = header_value('X-Discourse-Topic-Id') + post_id = header_value('X-Discourse-Post-Id') + reply_key = header_value('X-Discourse-Reply-Key') + + if topic_id.present? + email_log.topic_id = topic_id + + topic_identitfier = "" + @message.header['In-Reply-To'] = topic_identitfier + @message.header['References'] = topic_identitfier + end + + email_log.post_id = post_id if post_id.present? + email_log.reply_key = reply_key if reply_key.present? # Remove headers we don't need anymore @message.header['X-Discourse-Topic-Id'] = nil @@ -66,8 +79,7 @@ module Email end - def self.list_id_for(site_name, base_url) - + def self.host_for(base_url) host = "localhost" if base_url.present? begin @@ -76,18 +88,19 @@ module Email rescue URI::InvalidURIError end end + host + end + def self.list_id_for(site_name, host) "\"#{site_name.gsub(/\"/, "'")}\" " end private - def add_header_to_log(name, email_log, email_log_field) + def header_value(name) header = @message.header[name] - return unless header - - val = header.value - email_log[email_log_field] = val if val.present? + return nil unless header + header.value end end diff --git a/spec/components/email/sender_spec.rb b/spec/components/email/sender_spec.rb index abb1f2bf956..c13342b5379 100644 --- a/spec/components/email/sender_spec.rb +++ b/spec/components/email/sender_spec.rb @@ -20,32 +20,37 @@ describe Email::Sender do Email::Sender.new(message, :hello).send end + context "host_for" do + it "defaults to localhost" do + Email::Sender.host_for(nil).should == "localhost" + end + + it "returns localhost for a weird host" do + Email::Sender.host_for("this is not a real host").should == "localhost" + end + + it "parses hosts from urls" do + Email::Sender.host_for("http://meta.discourse.org").should == "meta.discourse.org" + end + + it "downcases hosts" do + Email::Sender.host_for("http://ForumSite.com").should == "forumsite.com" + end + + end + context "list_id_for" do - it "joins the host and forum name" do - Email::Sender.list_id_for("myforum", "http://mysite.com").should == '"myforum" ' - end - - it "uses localhost when no host is present" do - Email::Sender.list_id_for("myforum", nil).should == '"myforum" ' - end - - it "uses localhost with a weird host" do - Email::Sender.list_id_for("Fun", "this is not a real host").should == '"Fun" ' + Email::Sender.list_id_for("myforum", "mysite.com").should == '"myforum" ' end it "removes double quotes from names" do - Email::Sender.list_id_for('Quoted "Forum"', 'http://quoted.com').should == '"Quoted \'Forum\'" ' + Email::Sender.list_id_for('Quoted "Forum"', 'quoted.com').should == '"Quoted \'Forum\'" ' end it "converts the site name to lower case and removes spaces" do - Email::Sender.list_id_for("Robin's cool Forum!", "http://robin.com").should == '"Robin\'s cool Forum!" ' + Email::Sender.list_id_for("Robin's cool Forum!", "robin.com").should == '"Robin\'s cool Forum!" ' end - - it "downcases host names" do - Email::Sender.list_id_for("cool", "http://ForumSite.com").should == '"cool" ' - end - end context 'with a valid message' do @@ -92,6 +97,9 @@ describe Email::Sender do When { email_sender.send } Then { expect(email_log.post_id).to eq(3344) } Then { expect(email_log.topic_id).to eq(5577) } + Then { expect(message.header['In-Reply-To']).to be_present } + Then { expect(message.header['References']).to be_present } + end context "email log with a reply key" do