Adds Message-Id and References headers to email notifications so that issues and messages threads can be displayed by email clients (#1401).

git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@2281 e93f8b46-1217-0410-a6f0-8f06a7374b81

3 files changed, 85 insertions, 3 deletions

diff --git a/app/models/mailer.rb b/app/models/mailer.rb
index beef97a7b..0fd708879 100644
--- a/app/models/mailer.rb
+++ b/app/models/mailer.rb
@@ -27,6 +27,7 @@ class Mailer < ActionMailer::Base
                     'Issue-Id' => issue.id,
                     'Issue-Author' => issue.author.login
     redmine_headers 'Issue-Assignee' => issue.assigned_to.login if issue.assigned_to
+    message_id issue
     recipients issue.recipients
     cc(issue.watcher_recipients - @recipients)
     subject "[#{issue.project.name} - #{issue.tracker.name} ##{issue.id}] (#{issue.status.name}) #{issue.subject}"
@@ -40,6 +41,8 @@ class Mailer < ActionMailer::Base
                     'Issue-Id' => issue.id,
                     'Issue-Author' => issue.author.login
     redmine_headers 'Issue-Assignee' => issue.assigned_to.login if issue.assigned_to
+    message_id journal
+    references issue
     @author = journal.user
     recipients issue.recipients
     # Watchers in cc
@@ -95,6 +98,7 @@ class Mailer < ActionMailer::Base
 
   def news_added(news)
     redmine_headers 'Project' => news.project.identifier
+    message_id news
     recipients news.project.recipients
     subject "[#{news.project.name}] #{l(:label_news)}: #{news.title}"
     body :news => news,
@@ -104,6 +108,8 @@ class Mailer < ActionMailer::Base
   def message_posted(message, recipients)
     redmine_headers 'Project' => message.project.identifier,
                     'Topic-Id' => (message.parent_id || message.id)
+    message_id message
+    references message.parent unless message.parent.nil?
     recipients(recipients)
     subject "[#{message.board.project.name} - #{message.board.name}] #{message.subject}"
     body :message => message,
@@ -156,7 +162,15 @@ class Mailer < ActionMailer::Base
     return false if (recipients.nil? || recipients.empty?) &&
                     (cc.nil? || cc.empty?) &&
                     (bcc.nil? || bcc.empty?)
-    super
+                    
+    # Set Message-Id and References
+    if @message_id_object
+      mail.message_id = self.class.message_id_for(@message_id_object)
+    end
+    if @references_objects
+      mail.references = @references_objects.collect {|o| self.class.message_id_for(o)}
+    end
+    super(mail)
   end
 
   # Sends reminders to issue assignees
@@ -250,4 +264,34 @@ class Mailer < ActionMailer::Base
   def self.controller_path
     ''
   end unless respond_to?('controller_path')
+  
+  # Returns a predictable Message-Id for the given object
+  def self.message_id_for(object)
+    # id + timestamp should reduce the odds of a collision
+    # as far as we don't send multiple emails for the same object
+    hash = "redmine.#{object.class.name.demodulize.underscore}-#{object.id}.#{object.created_on.strftime("%Y%m%d%H%M%S")}"
+    host = Setting.mail_from.to_s.gsub(%r{^.*@}, '')
+    host = "#{::Socket.gethostname}.redmine" if host.empty?
+    "<#{hash}@#{host}>"
+  end
+  
+  private
+  
+  def message_id(object)
+    @message_id_object = object
+  end
+  
+  def references(object)
+    @references_objects ||= []
+    @references_objects << object
+  end
+end
+
+# Patch TMail so that message_id is not overwritten
+module TMail
+  class Mail
+    def add_message_id( fqdn = nil )
+      self.message_id ||= ::TMail::new_message_id(fqdn)
+    end
+  end
 end
diff --git a/test/fixtures/messages.yml b/test/fixtures/messages.yml
index e578853d4..8ed376eb2 100644
--- a/test/fixtures/messages.yml
+++ b/test/fixtures/messages.yml
@@ -30,7 +30,7 @@ messages_003:
   replies_count: 0

   last_reply_id: 

   content: "An other reply"

-  author_id: 

+  author_id: 2

   parent_id: 1

   board_id: 1

 messages_004: 

diff --git a/test/unit/mailer_test.rb b/test/unit/mailer_test.rb
index aded23cc3..1e09eaa5c 100644
--- a/test/unit/mailer_test.rb
+++ b/test/unit/mailer_test.rb
@@ -95,7 +95,45 @@ class MailerTest < Test::Unit::TestCase
     assert !mail.body.include?('<a href="https://mydomain.foo/issues/show/1">Bug #1: Can\'t print recipes</a>')
   end
   
-
+  def test_issue_add_message_id
+    ActionMailer::Base.deliveries.clear
+    issue = Issue.find(1)
+    Mailer.deliver_issue_add(issue)
+    mail = ActionMailer::Base.deliveries.last
+    assert_not_nil mail
+    assert_equal Mailer.message_id_for(issue), mail.message_id
+    assert_nil mail.references
+  end
+  
+  def test_issue_edit_message_id
+    ActionMailer::Base.deliveries.clear
+    journal = Journal.find(1)
+    Mailer.deliver_issue_edit(journal)
+    mail = ActionMailer::Base.deliveries.last
+    assert_not_nil mail
+    assert_equal Mailer.message_id_for(journal), mail.message_id
+    assert_equal Mailer.message_id_for(journal.issue), mail.references.to_s
+  end
+  
+  def test_message_posted_message_id
+    ActionMailer::Base.deliveries.clear
+    message = Message.find(1)
+    Mailer.deliver_message_posted(message, message.author.mail)
+    mail = ActionMailer::Base.deliveries.last
+    assert_not_nil mail
+    assert_equal Mailer.message_id_for(message), mail.message_id
+    assert_nil mail.references
+  end
+  
+  def test_reply_posted_message_id
+    ActionMailer::Base.deliveries.clear
+    message = Message.find(3)
+    Mailer.deliver_message_posted(message, message.author.mail)
+    mail = ActionMailer::Base.deliveries.last
+    assert_not_nil mail
+    assert_equal Mailer.message_id_for(message), mail.message_id
+    assert_equal Mailer.message_id_for(message.parent), mail.references.to_s
+  end
   
   # test mailer methods for each language
   def test_issue_add