Issue emails cannot be threaded by some mailers due to inconsistent Message-ID...

Issue emails cannot be threaded by some mailers due to inconsistent Message-ID and References field (#17096).

Patch by Go MAEDA.


git-svn-id: https://svn.redmine.org/redmine/trunk@17927 e93f8b46-1217-0410-a6f0-8f06a7374b81

app/models/mailer.rb

@@ -137,6 +137,7 @@ class Mailer < ActionMailer::Base
     redmine_headers 'Project' => document.project.identifier
     @author = author
     @document = document
+    @user = user
     @document_url = url_for(:controller => 'documents', :action => 'show', :id => document)
     mail :to => user,
       :subject => "[#{document.project.name}] #{l(:label_document_new)}: #{document.title}"
@@ -172,6 +173,7 @@ class Mailer < ActionMailer::Base
     end
     redmine_headers 'Project' => container.project.identifier
     @attachments = attachments
+    @user = user
     @added_to = added_to
     @added_to_url = added_to_url
     mail :to => user,
@@ -203,6 +205,7 @@ class Mailer < ActionMailer::Base
     message_id news
     references news
     @news = news
+    @user = user
     @news_url = url_for(:controller => 'news', :action => 'show', :id => news)
     mail :to => user,
       :subject => "[#{news.project.name}] #{l(:label_news)}: #{news.title}"
@@ -228,6 +231,7 @@ class Mailer < ActionMailer::Base
     references news
     @news = news
     @comment = comment
+    @user = user
     @news_url = url_for(:controller => 'news', :action => 'show', :id => news)
     mail :to => user,
      :subject => "Re: [#{news.project.name}] #{l(:label_news)}: #{news.title}"
@@ -253,6 +257,7 @@ class Mailer < ActionMailer::Base
     message_id message
     references message.root
     @message = message
+    @user = user
     @message_url = url_for(message.event_url)
     mail :to => user,
       :subject => "[#{message.board.project.name} - #{message.board.name} - msg#{message.root.id}] #{message.subject}"
@@ -279,6 +284,7 @@ class Mailer < ActionMailer::Base
     @author = wiki_content.author
     message_id wiki_content
     @wiki_content = wiki_content
+    @user = user
     @wiki_content_url = url_for(:controller => 'wiki', :action => 'show',
                                       :project_id => wiki_content.project,
                                       :id => wiki_content.page.title)
@@ -304,6 +310,7 @@ class Mailer < ActionMailer::Base
     @author = wiki_content.author
     message_id wiki_content
     @wiki_content = wiki_content
+    @user = user
     @wiki_content_url = url_for(:controller => 'wiki', :action => 'show',
                                       :project_id => wiki_content.project,
                                       :id => wiki_content.page.title)
@@ -662,10 +669,10 @@ class Mailer < ActionMailer::Base
     end
 
     if @message_id_object
-      headers[:message_id] = "<#{self.class.message_id_for(@message_id_object)}>"
+      headers[:message_id] = "<#{self.class.message_id_for(@message_id_object, @user)}>"
     end
     if @references_objects
-      headers[:references] = @references_objects.collect {|o| "<#{self.class.references_for(o)}>"}.join(' ')
+      headers[:references] = @references_objects.collect {|o| "<#{self.class.references_for(o, @user)}>"}.join(' ')
     end
 
     if block_given?
@@ -719,30 +726,28 @@ class Mailer < ActionMailer::Base
     h.each { |k,v| headers["X-Redmine-#{k}"] = v.to_s }
   end
 
-  def self.token_for(object, rand=true)
+  def self.token_for(object, user)
     timestamp = object.send(object.respond_to?(:created_on) ? :created_on : :updated_on)
     hash = [
       "redmine",
       "#{object.class.name.demodulize.underscore}-#{object.id}",
       timestamp.utc.strftime("%Y%m%d%H%M%S")
     ]
-    if rand
-      hash << Redmine::Utils.random_hex(8)
-    end
+    hash << user.id if user
     host = Setting.mail_from.to_s.strip.gsub(%r{^.*@|>}, '')
     host = "#{::Socket.gethostname}.redmine" if host.empty?
     "#{hash.join('.')}@#{host}"
   end
 
   # Returns a Message-Id for the given object
-  def self.message_id_for(object)
-    token_for(object, true)
+  def self.message_id_for(object, user)
+    token_for(object, user)
   end
 
   # Returns a uniq token for a given object referenced by all notifications
   # related to this object
-  def self.references_for(object)
-    token_for(object, false)
+  def self.references_for(object, user)
+    token_for(object, user)
   end
 
   def message_id(object)
@@ -754,3 +759,4 @@ class Mailer < ActionMailer::Base
     @references_objects << object
   end
 end
+

test/unit/mailer_test.rb

@@ -296,8 +296,9 @@ class MailerTest < ActiveSupport::TestCase
     issue = Issue.find(2)
     Mailer.deliver_issue_add(issue)
     mail = last_email
-    assert_match /^redmine\.issue-2\.20060719190421\.[a-f0-9]+@example\.net/, mail.message_id
-    assert_include "redmine.issue-2.20060719190421@example.net", mail.references
+    uid = destination_user(mail).id
+    assert_include "redmine.issue-2.20060719190421.#{uid}@example.net", mail.message_id
+    assert_include "redmine.issue-2.20060719190421.#{uid}@example.net", mail.references
   end
 
   def test_issue_edit_message_id
@@ -306,8 +307,9 @@ class MailerTest < ActiveSupport::TestCase
 
     Mailer.deliver_issue_edit(journal)
     mail = last_email
-    assert_match /^redmine\.journal-3\.\d+\.[a-f0-9]+@example\.net/, mail.message_id
-    assert_include "redmine.issue-2.20060719190421@example.net", mail.references
+    uid = destination_user(mail).id
+    assert_match /^redmine\.journal-3\.\d+\.#{uid}@example\.net/, mail.message_id
+    assert_include "redmine.issue-2.20060719190421.#{uid}@example.net", mail.references
     assert_select_email do
       # link to the update
       assert_select "a[href=?]",
@@ -319,8 +321,9 @@ class MailerTest < ActiveSupport::TestCase
     message = Message.find(1)
     Mailer.deliver_message_posted(message)
     mail = last_email
-    assert_match /^redmine\.message-1\.\d+\.[a-f0-9]+@example\.net/, mail.message_id
-    assert_include "redmine.message-1.20070512151532@example.net", mail.references
+    uid = destination_user(mail).id
+    assert_include "redmine.message-1.20070512151532.#{uid}@example.net", mail.message_id
+    assert_include "redmine.message-1.20070512151532.#{uid}@example.net", mail.references
     assert_select_email do
       # link to the message
       assert_select "a[href=?]",
@@ -333,8 +336,9 @@ class MailerTest < ActiveSupport::TestCase
     message = Message.find(3)
     Mailer.deliver_message_posted(message)
     mail = last_email
-    assert_match /^redmine\.message-3\.\d+\.[a-f0-9]+@example\.net/, mail.message_id
-    assert_include "redmine.message-1.20070512151532@example.net", mail.references
+    uid = destination_user(mail).id
+    assert_include "redmine.message-3.20070512151802.#{uid}@example.net", mail.message_id
+    assert_include "redmine.message-1.20070512151532.#{uid}@example.net", mail.references
     assert_select_email do
       # link to the reply
       assert_select "a[href=?]",
@@ -346,9 +350,10 @@ class MailerTest < ActiveSupport::TestCase
   def test_timestamp_in_message_id_should_be_utc
     zone_was = Time.zone
     issue = Issue.find(3)
+    user = User.find(1)
     %w(UTC Paris Tokyo).each do |zone|
       Time.zone = zone
-      assert_match /^redmine\.issue-3\.20060719190727\.[a-f0-9]+@example\.net/, Mailer.token_for(issue)
+      assert_match /^redmine\.issue-3\.20060719190727\.1@example\.net/, Mailer.token_for(issue, user)
     end
   ensure
     Time.zone = zone_was
@@ -808,7 +813,8 @@ class MailerTest < ActiveSupport::TestCase
 
   def test_token_for_should_strip_trailing_gt_from_address_with_full_name
     with_settings :mail_from => "Redmine Mailer<no-reply@redmine.org>" do
-      assert_match /\Aredmine.issue-\d+\.\d+\.[0-9a-f]+@redmine.org\z/, Mailer.token_for(Issue.generate!)
+      assert_match /\Aredmine.issue-\d+\.\d+\.3@redmine.org\z/,
+        Mailer.token_for(Issue.generate!, User.find(3))
     end
   end
 
@@ -960,4 +966,8 @@ class MailerTest < ActiveSupport::TestCase
   def html_part
     last_email.parts.detect {|part| part.content_type.include?('text/html')}
   end
+
+  def destination_user(mail)
+    EmailAddress.where(:address => [mail.to, mail.cc, mail.bcc].flatten).map(&:user).first
+  end
 end