From 260e8b84f8544f5372e2269a76893f11fd9ad3c0 Mon Sep 17 00:00:00 2001
From: Eric Davis <edavis@littlestreamsoftware.com>
Date: Thu, 24 Feb 2011 11:10:23 -0800
Subject: [PATCH] [#674] Convert outbound mail to be sent per-recipient

Instead of a single email that is sent out with all the recipients as CC/BCC,
each recipient will be delivered their own email. This will let emails to be
customized per user based on their permissions, without exposing private data.
---
 app/controllers/documents_controller.rb      |   7 +-
 app/controllers/files_controller.rb          |   6 +-
 app/models/document_observer.rb              |   6 +-
 app/models/issue_observer.rb                 |   4 +-
 app/models/journal_observer.rb               |   5 +-
 app/models/mailer.rb                         |  61 ++++-------
 app/models/message_observer.rb               |   9 +-
 app/models/news_observer.rb                  |   6 +-
 app/models/wiki_content_observer.rb          |  13 ++-
 test/functional/admin_controller_test.rb     |   2 +-
 test/functional/documents_controller_test.rb |   2 +-
 test/functional/issues_controller_test.rb    |  13 +--
 test/functional/messages_controller_test.rb  |  13 ++-
 test/functional/news_controller_test.rb      |   2 +-
 test/functional/users_controller_test.rb     |   6 +-
 test/unit/changeset_test.rb                  |   2 +-
 test/unit/document_test.rb                   |   2 +-
 test/unit/issue_test.rb                      |   4 +-
 test/unit/journal_observer_test.rb           |   8 +-
 test/unit/journal_test.rb                    |   2 +-
 test/unit/lib/redmine/hook_test.rb           |   4 +-
 test/unit/mail_handler_test.rb               |   2 +-
 test/unit/mailer_test.rb                     | 109 +++++++------------
 test/unit/message_test.rb                    |   4 +-
 test/unit/news_test.rb                       |   2 +-
 test/unit/repository_test.rb                 |   6 +-
 test/unit/wiki_content_test.rb               |   8 +-
 27 files changed, 149 insertions(+), 159 deletions(-)

diff --git a/app/controllers/documents_controller.rb b/app/controllers/documents_controller.rb
index 776b4759..39906610 100644
--- a/app/controllers/documents_controller.rb
+++ b/app/controllers/documents_controller.rb
@@ -69,7 +69,12 @@ class DocumentsController < ApplicationController
     attachments = Attachment.attach_files(@document, params[:attachments])
     render_attachment_warning_if_needed(@document)
 
-    Mailer.deliver_attachments_added(attachments[:files]) if attachments.present? && attachments[:files].present? && Setting.notified_events.include?('document_added')
+    if attachments.present? && attachments[:files].present? && Setting.notified_events.include?('document_added')
+      # TODO: refactor
+      attachments.first.container.recipients.each do |recipient|
+        Mailer.deliver_attachments_added(attachments[:files], recipient)
+      end
+    end
     redirect_to :action => 'show', :id => @document
   end
 
diff --git a/app/controllers/files_controller.rb b/app/controllers/files_controller.rb
index 0114ea84..d3be7791 100644
--- a/app/controllers/files_controller.rb
+++ b/app/controllers/files_controller.rb
@@ -42,7 +42,11 @@ class FilesController < ApplicationController
     render_attachment_warning_if_needed(container)
 
     if !attachments.empty? && !attachments[:files].blank? && Setting.notified_events.include?('file_added')
-      Mailer.deliver_attachments_added(attachments[:files])
+      # TODO: refactor
+      recipients = attachments[:files].first.container.project.notified_users.select {|user| user.allowed_to?(:view_files, container.project)}.collect  {|u| u.mail}
+      recipients.each do |recipient|
+        Mailer.deliver_attachments_added(attachments[:files], recipient)
+      end
     end
     redirect_to project_files_path(@project)
   end
diff --git a/app/models/document_observer.rb b/app/models/document_observer.rb
index 66ac3a8a..615f193a 100644
--- a/app/models/document_observer.rb
+++ b/app/models/document_observer.rb
@@ -14,6 +14,10 @@
 
 class DocumentObserver < ActiveRecord::Observer
   def after_create(document)
-    Mailer.deliver_document_added(document) if Setting.notified_events.include?('document_added')
+    if Setting.notified_events.include?('document_added')
+      document.recipients.each do |recipient|
+        Mailer.deliver_document_added(document, recipient)
+      end
+    end
   end
 end
diff --git a/app/models/issue_observer.rb b/app/models/issue_observer.rb
index 38d7fd29..39092053 100644
--- a/app/models/issue_observer.rb
+++ b/app/models/issue_observer.rb
@@ -17,7 +17,9 @@ class IssueObserver < ActiveRecord::Observer
 
   def after_create(issue)
     if self.send_notification
-      Mailer.deliver_issue_add(issue) if Setting.notified_events.include?('issue_added')
+      (issue.recipients + issue.watcher_recipients).uniq.each do |recipient|
+        Mailer.deliver_issue_add(issue, recipient)
+      end
     end
     clear_notification
   end
diff --git a/app/models/journal_observer.rb b/app/models/journal_observer.rb
index 32f17505..708132e5 100644
--- a/app/models/journal_observer.rb
+++ b/app/models/journal_observer.rb
@@ -27,7 +27,10 @@ class JournalObserver < ActiveRecord::Observer
         (Setting.notified_events.include?('issue_note_added') && journal.notes.present?) ||
         (Setting.notified_events.include?('issue_status_updated') && journal.new_status.present?) ||
         (Setting.notified_events.include?('issue_priority_updated') && journal.new_value_for('priority_id').present?)
-      Mailer.deliver_issue_edit(journal)
+      issue = journal.issue
+      (issue.recipients + issue.watcher_recipients).uniq.each do |recipient|
+        Mailer.deliver_issue_edit(journal, recipient)
+      end
     end
   end
 
diff --git a/app/models/mailer.rb b/app/models/mailer.rb
index 8e331011..c4bba852 100644
--- a/app/models/mailer.rb
+++ b/app/models/mailer.rb
@@ -30,20 +30,19 @@ class Mailer < ActionMailer::Base
     { :host => h, :protocol => Setting.protocol }
   end
 
-  # Builds a tmail object used to email recipients of the added issue.
+  # Builds a tmail object used to email a recipient of the added issue.
   #
   # Example:
-  #   issue_add(issue) => tmail object
-  #   Mailer.deliver_issue_add(issue) => sends an email to issue recipients
-  def issue_add(issue)
+  #   issue_add(issue, 'user@example.com') => tmail object
+  #   Mailer.deliver_issue_add(issue, 'user@example.com') => sends an email to 'user@example.com'
+  def issue_add(issue, recipient)
     redmine_headers 'Project' => issue.project.identifier,
                     'Issue-Id' => issue.id,
                     'Issue-Author' => issue.author.login,
                     'Type' => "Issue"
     redmine_headers 'Issue-Assignee' => issue.assigned_to.login if issue.assigned_to
     message_id issue
-    recipients issue.recipients
-    cc(issue.watcher_recipients - @recipients)
+    recipients [recipient]
     subject "[#{issue.project.name} - #{issue.tracker.name} ##{issue.id}] (#{issue.status.name}) #{issue.subject}"
     body :issue => issue,
          :issue_url => url_for(:controller => 'issues', :action => 'show', :id => issue)
@@ -53,9 +52,9 @@ class Mailer < ActionMailer::Base
   # Builds a tmail object used to email recipients of the edited issue.
   #
   # Example:
-  #   issue_edit(journal) => tmail object
-  #   Mailer.deliver_issue_edit(journal) => sends an email to issue recipients
-  def issue_edit(journal)
+  #   issue_edit(journal, 'user@example.com') => tmail object
+  #   Mailer.deliver_issue_edit(journal, 'user@example.com') => sends an email to issue recipients
+  def issue_edit(journal, recipient)
     issue = journal.journaled.reload
     redmine_headers 'Project' => issue.project.identifier,
                     'Issue-Id' => issue.id,
@@ -65,9 +64,7 @@ class Mailer < ActionMailer::Base
     message_id journal
     references issue
     @author = journal.user
-    recipients issue.recipients
-    # Watchers in cc
-    cc(issue.watcher_recipients - @recipients)
+    recipients [recipient]
     s = "[#{issue.project.name} - #{issue.tracker.name} ##{issue.id}] "
     s << "(#{issue.status.name}) " if journal.details['status_id']
     s << issue.subject
@@ -93,12 +90,12 @@ class Mailer < ActionMailer::Base
   # Builds a tmail object used to email users belonging to the added document's project.
   #
   # Example:
-  #   document_added(document) => tmail object
-  #   Mailer.deliver_document_added(document) => sends an email to the document's project recipients
-  def document_added(document)
+  #   document_added(document, 'test@example.com') => tmail object
+  #   Mailer.deliver_document_added(document, 'test@example.com') => sends an email to the document's project recipients
+  def document_added(document, recipient)
     redmine_headers 'Project' => document.project.identifier,
                     'Type' => "Document"
-    recipients document.recipients
+    recipients [recipient]
     subject "[#{document.project.name}] #{l(:label_document_new)}: #{document.title}"
     body :document => document,
          :document_url => url_for(:controller => 'documents', :action => 'show', :id => document)
@@ -110,7 +107,7 @@ class Mailer < ActionMailer::Base
   # Example:
   #   attachments_added(attachments) => tmail object
   #   Mailer.deliver_attachments_added(attachments) => sends an email to the project's recipients
-  def attachments_added(attachments)
+  def attachments_added(attachments, recipient)
     container = attachments.first.container
     added_to = ''
     added_to_url = ''
@@ -118,16 +115,14 @@ class Mailer < ActionMailer::Base
     when 'Project'
       added_to_url = url_for(:controller => 'files', :action => 'index', :project_id => container)
       added_to = "#{l(:label_project)}: #{container}"
-      recipients container.project.notified_users.select {|user| user.allowed_to?(:view_files, container.project)}.collect  {|u| u.mail}
     when 'Version'
       added_to_url = url_for(:controller => 'files', :action => 'index', :project_id => container.project)
       added_to = "#{l(:label_version)}: #{container.name}"
-      recipients container.project.notified_users.select {|user| user.allowed_to?(:view_files, container.project)}.collect  {|u| u.mail}
     when 'Document'
       added_to_url = url_for(:controller => 'documents', :action => 'show', :id => container.id)
       added_to = "#{l(:label_document)}: #{container.title}"
-      recipients container.recipients
     end
+    recipients [recipient]
     redmine_headers 'Project' => container.project.identifier,
                     'Type' => "Attachment"
     subject "[#{container.project.name}] #{l(:label_attachment_new)}"
@@ -142,11 +137,11 @@ class Mailer < ActionMailer::Base
   # Example:
   #   news_added(news) => tmail object
   #   Mailer.deliver_news_added(news) => sends an email to the news' project recipients
-  def news_added(news)
+  def news_added(news, recipient)
     redmine_headers 'Project' => news.project.identifier,
                     'Type' => "News"
     message_id news
-    recipients news.recipients
+    recipients [recipient]
     subject "[#{news.project.name}] #{l(:label_news)}: #{news.title}"
     body :news => news,
          :news_url => url_for(:controller => 'news', :action => 'show', :id => news)
@@ -176,14 +171,13 @@ class Mailer < ActionMailer::Base
   # Example:
   #   message_posted(message) => tmail object
   #   Mailer.deliver_message_posted(message) => sends an email to the recipients
-  def message_posted(message)
+  def message_posted(message, recipient)
     redmine_headers 'Project' => message.project.identifier,
                     'Topic-Id' => (message.parent_id || message.id),
                     'Type' => "Forum"
     message_id message
     references message.parent unless message.parent.nil?
-    recipients(message.recipients)
-    cc((message.root.watcher_recipients + message.board.watcher_recipients).uniq - @recipients)
+    recipients [recipient]
     subject "[#{message.board.project.name} - #{message.board.name} - msg#{message.root.id}] #{message.subject}"
     body :message => message,
          :message_url => url_for({ :controller => 'messages', :action => 'show', :board_id => message.board, :id => message.root, :r => message, :anchor => "message-#{message.id}" })
@@ -195,13 +189,12 @@ class Mailer < ActionMailer::Base
   # Example:
   #   wiki_content_added(wiki_content) => tmail object
   #   Mailer.deliver_wiki_content_added(wiki_content) => sends an email to the project's recipients
-  def wiki_content_added(wiki_content)
+  def wiki_content_added(wiki_content, recipient)
     redmine_headers 'Project' => wiki_content.project.identifier,
                     'Wiki-Page-Id' => wiki_content.page.id,
                     'Type' => "Wiki"
     message_id wiki_content
-    recipients wiki_content.recipients
-    cc(wiki_content.page.wiki.watcher_recipients - recipients)
+    recipients [recipient]
     subject "[#{wiki_content.project.name}] #{l(:mail_subject_wiki_content_added, :id => wiki_content.page.pretty_title)}"
     body :wiki_content => wiki_content,
          :wiki_content_url => url_for(:controller => 'wiki', :action => 'show', :project_id => wiki_content.project, :id => wiki_content.page.title)
@@ -213,13 +206,12 @@ class Mailer < ActionMailer::Base
   # Example:
   #   wiki_content_updated(wiki_content) => tmail object
   #   Mailer.deliver_wiki_content_updated(wiki_content) => sends an email to the project's recipients
-  def wiki_content_updated(wiki_content)
+  def wiki_content_updated(wiki_content, recipient)
     redmine_headers 'Project' => wiki_content.project.identifier,
                     'Wiki-Page-Id' => wiki_content.page.id,
                     'Type' => "Wiki"
     message_id wiki_content
-    recipients wiki_content.recipients
-    cc(wiki_content.page.wiki.watcher_recipients + wiki_content.page.watcher_recipients - recipients)
+    recipients [recipient]
     subject "[#{wiki_content.project.name}] #{l(:mail_subject_wiki_content_updated, :id => wiki_content.page.pretty_title)}"
     body :wiki_content => wiki_content,
          :wiki_content_url => url_for(:controller => 'wiki', :action => 'show', :project_id => wiki_content.project, :id => wiki_content.page.title),
@@ -403,13 +395,6 @@ class Mailer < ActionMailer::Base
     notified_users = [recipients, cc].flatten.compact.uniq
     # Rails would log recipients only, not cc and bcc
     mylogger.info "Sending email notification to: #{notified_users.join(', ')}" if mylogger
-
-    # Blind carbon copy recipients
-    if Setting.bcc_recipients?
-      bcc(notified_users)
-      recipients []
-      cc []
-    end
     super
   end
 
diff --git a/app/models/message_observer.rb b/app/models/message_observer.rb
index 59f31a9d..780b23ff 100644
--- a/app/models/message_observer.rb
+++ b/app/models/message_observer.rb
@@ -14,6 +14,13 @@
 
 class MessageObserver < ActiveRecord::Observer
   def after_create(message)
-    Mailer.deliver_message_posted(message) if Setting.notified_events.include?('message_posted')
+    if Setting.notified_events.include?('message_posted')
+      recipients = message.recipients
+      recipients += message.root.watcher_recipients
+      recipients += message.board.watcher_recipients
+      recipients.uniq.each do |recipient|
+        Mailer.deliver_message_posted(message, recipient)
+      end
+    end
   end
 end
diff --git a/app/models/news_observer.rb b/app/models/news_observer.rb
index bbf1c347..3feab7b2 100644
--- a/app/models/news_observer.rb
+++ b/app/models/news_observer.rb
@@ -14,6 +14,10 @@
 
 class NewsObserver < ActiveRecord::Observer
   def after_create(news)
-    Mailer.deliver_news_added(news) if Setting.notified_events.include?('news_added')
+    if Setting.notified_events.include?('news_added')
+      news.recipients.each do |recipient|
+        Mailer.deliver_news_added(news, recipient)
+      end
+    end
   end
 end
diff --git a/app/models/wiki_content_observer.rb b/app/models/wiki_content_observer.rb
index aeb8b587..4715fbe1 100644
--- a/app/models/wiki_content_observer.rb
+++ b/app/models/wiki_content_observer.rb
@@ -14,12 +14,19 @@
 
 class WikiContentObserver < ActiveRecord::Observer
   def after_create(wiki_content)
-    Mailer.deliver_wiki_content_added(wiki_content) if Setting.notified_events.include?('wiki_content_added')
+    if Setting.notified_events.include?('wiki_content_added')
+      (wiki_content.recipients + wiki_content.page.wiki.watcher_recipients).uniq.each do |recipient|
+        Mailer.deliver_wiki_content_added(wiki_content, recipient)
+      end
+    end
   end
 
   def after_update(wiki_content)
-    if wiki_content.text_changed?
-      Mailer.deliver_wiki_content_updated(wiki_content) if Setting.notified_events.include?('wiki_content_updated')
+    if wiki_content.text_changed? && Setting.notified_events.include?('wiki_content_updated')
+
+      (wiki_content.recipients + wiki_content.page.wiki.watcher_recipients + wiki_content.page.watcher_recipients).uniq.each do |recipient|
+        Mailer.deliver_wiki_content_updated(wiki_content, recipient)
+      end
     end
   end
 end
diff --git a/test/functional/admin_controller_test.rb b/test/functional/admin_controller_test.rb
index 8569565a..a4bb057f 100644
--- a/test/functional/admin_controller_test.rb
+++ b/test/functional/admin_controller_test.rb
@@ -74,7 +74,7 @@ class AdminControllerTest < ActionController::TestCase
     mail = ActionMailer::Base.deliveries.last
     assert_kind_of TMail::Mail, mail
     user = User.find(1)
-    assert_equal [user.mail], mail.bcc
+    assert_equal [user.mail], mail.to
   end
 
   def test_no_plugins
diff --git a/test/functional/documents_controller_test.rb b/test/functional/documents_controller_test.rb
index d3c5d457..23eaf8e1 100644
--- a/test/functional/documents_controller_test.rb
+++ b/test/functional/documents_controller_test.rb
@@ -80,7 +80,7 @@ LOREM
     assert_equal Enumeration.find(2), document.category
     assert_equal 1, document.attachments.size
     assert_equal 'testfile.txt', document.attachments.first.filename
-    assert_equal 1, ActionMailer::Base.deliveries.size
+    assert_equal 2, ActionMailer::Base.deliveries.size
   end
 
   def test_destroy
diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb
index 84c1e94b..0c4e575d 100644
--- a/test/functional/issues_controller_test.rb
+++ b/test/functional/issues_controller_test.rb
@@ -521,9 +521,8 @@ class IssuesControllerTest < ActionController::TestCase
     assert_equal [2, 3], issue.watcher_user_ids.sort
     assert issue.watched_by?(User.find(3))
     # Watchers notified
-    mail = ActionMailer::Base.deliveries.last
-    assert_kind_of TMail::Mail, mail
-    assert [mail.bcc, mail.cc].flatten.include?(User.find(3).mail)
+    recipients = ActionMailer::Base.deliveries.collect(&:to)
+    assert recipients.flatten.include?(User.find(3).mail)
   end
 
   def test_post_create_subissue
@@ -567,8 +566,8 @@ class IssuesControllerTest < ActionController::TestCase
                             :custom_field_values => {'2' => 'Value for field 2'}}
     end
     assert_redirected_to :controller => 'issues', :action => 'show', :id => Issue.last.id
-
-    assert_equal 1, ActionMailer::Base.deliveries.size
+    
+    assert_equal 2, ActionMailer::Base.deliveries.size
   end
 
   def test_post_create_should_preserve_fields_values_on_validation_failure
@@ -1017,7 +1016,7 @@ class IssuesControllerTest < ActionController::TestCase
                                      :priority_id => '6',
                                      :category_id => '1' # no change
                                     }
-    assert_equal 1, ActionMailer::Base.deliveries.size
+    assert_equal 2, ActionMailer::Base.deliveries.size
   end
 
   def test_put_update_should_not_send_a_notification_if_send_notification_is_off
@@ -1240,7 +1239,7 @@ class IssuesControllerTest < ActionController::TestCase
          })
 
     assert_response 302
-    assert_equal 2, ActionMailer::Base.deliveries.size
+    assert_equal 5, ActionMailer::Base.deliveries.size
   end
 
   def test_bulk_update_status
diff --git a/test/functional/messages_controller_test.rb b/test/functional/messages_controller_test.rb
index 9b34159a..be8b04a2 100644
--- a/test/functional/messages_controller_test.rb
+++ b/test/functional/messages_controller_test.rb
@@ -88,14 +88,19 @@ class MessagesControllerTest < ActionController::TestCase
     assert_equal 2, message.author_id
     assert_equal 1, message.board_id
 
-    mail = ActionMailer::Base.deliveries.last
+    # author
+    mails_to_author = ActionMailer::Base.deliveries.select {|m| m.to.include?('jsmith@somenet.foo') }
+    assert_equal 1, mails_to_author.length
+    mail = mails_to_author.first
+    assert mail.to.include?('jsmith@somenet.foo')
     assert_kind_of TMail::Mail, mail
     assert_equal "[#{message.board.project.name} - #{message.board.name} - msg#{message.root.id}] Test created message", mail.subject
     assert mail.body.include?('Message body')
-    # author
-    assert mail.bcc.include?('jsmith@somenet.foo')
+
     # project member
-    assert mail.bcc.include?('dlopper@somenet.foo')
+    mails_to_member = ActionMailer::Base.deliveries.select {|m| m.to.include?('dlopper@somenet.foo') }
+    assert_equal 1, mails_to_member.length
+    assert mails_to_member.first.to.include?('dlopper@somenet.foo')
   end
 
   def test_get_edit
diff --git a/test/functional/news_controller_test.rb b/test/functional/news_controller_test.rb
index bd2dbde0..c19b7b8e 100644
--- a/test/functional/news_controller_test.rb
+++ b/test/functional/news_controller_test.rb
@@ -76,7 +76,7 @@ class NewsControllerTest < ActionController::TestCase
     assert_equal 'This is the description', news.description
     assert_equal User.find(2), news.author
     assert_equal Project.find(1), news.project
-    assert_equal 1, ActionMailer::Base.deliveries.size
+    assert_equal 2, ActionMailer::Base.deliveries.size
   end
 
   def test_get_edit
diff --git a/test/functional/users_controller_test.rb b/test/functional/users_controller_test.rb
index d97291fa..bbc0de39 100644
--- a/test/functional/users_controller_test.rb
+++ b/test/functional/users_controller_test.rb
@@ -179,7 +179,7 @@ class UsersControllerTest < ActionController::TestCase
 
     mail = ActionMailer::Base.deliveries.last
     assert_not_nil mail
-    assert_equal [user.mail], mail.bcc
+    assert_equal [user.mail], mail.to
     assert mail.body.include?('secret')
   end
 
@@ -240,7 +240,7 @@ class UsersControllerTest < ActionController::TestCase
     assert u.reload.active?
     mail = ActionMailer::Base.deliveries.last
     assert_not_nil mail
-    assert_equal ['foo.bar@somenet.foo'], mail.bcc
+    assert_equal ['foo.bar@somenet.foo'], mail.to
     assert mail.body.include?(ll('fr', :notice_account_activated))
   end
 
@@ -254,7 +254,7 @@ class UsersControllerTest < ActionController::TestCase
 
     mail = ActionMailer::Base.deliveries.last
     assert_not_nil mail
-    assert_equal [u.mail], mail.bcc
+    assert_equal [u.mail], mail.to
     assert mail.body.include?('newpass')
   end
 
diff --git a/test/unit/changeset_test.rb b/test/unit/changeset_test.rb
index 43951267..d05f050b 100644
--- a/test/unit/changeset_test.rb
+++ b/test/unit/changeset_test.rb
@@ -37,7 +37,7 @@ class ChangesetTest < ActiveSupport::TestCase
     fixed = Issue.find(1)
     assert fixed.closed?
     assert_equal 90, fixed.done_ratio
-    assert_equal 1, ActionMailer::Base.deliveries.size
+    assert_equal 2, ActionMailer::Base.deliveries.size
   end
 
   def test_ref_keywords
diff --git a/test/unit/document_test.rb b/test/unit/document_test.rb
index a82a6233..c3e161d6 100644
--- a/test/unit/document_test.rb
+++ b/test/unit/document_test.rb
@@ -27,7 +27,7 @@ class DocumentTest < ActiveSupport::TestCase
     doc = Document.new(:project => Project.find(1), :title => 'New document', :category => Enumeration.find_by_name('User documentation'))
 
     assert doc.save
-    assert_equal 1, ActionMailer::Base.deliveries.size
+    assert_equal 2, ActionMailer::Base.deliveries.size
   end
 
   def test_create_with_default_category
diff --git a/test/unit/issue_test.rb b/test/unit/issue_test.rb
index 448cadbb..73a6de7d 100644
--- a/test/unit/issue_test.rb
+++ b/test/unit/issue_test.rb
@@ -599,7 +599,7 @@ class IssueTest < ActiveSupport::TestCase
     issue = Issue.new(:project_id => 1, :tracker_id => 1, :author_id => 3, :status_id => 1, :priority => IssuePriority.all.first, :subject => 'test_create', :estimated_hours => '1:30')
 
     assert issue.save
-    assert_equal 1, ActionMailer::Base.deliveries.size
+    assert_equal 2, ActionMailer::Base.deliveries.size
   end
 
   def test_stale_issue_should_not_send_email_notification
@@ -610,7 +610,7 @@ class IssueTest < ActiveSupport::TestCase
     issue.init_journal(User.find(1))
     issue.subject = 'Subjet update'
     assert issue.save
-    assert_equal 1, ActionMailer::Base.deliveries.size
+    assert_equal 2, ActionMailer::Base.deliveries.size
     ActionMailer::Base.deliveries.clear
 
     stale.init_journal(User.find(1))
diff --git a/test/unit/journal_observer_test.rb b/test/unit/journal_observer_test.rb
index 01ccaf31..eb6e9c30 100644
--- a/test/unit/journal_observer_test.rb
+++ b/test/unit/journal_observer_test.rb
@@ -25,7 +25,7 @@ class JournalObserverTest < ActiveSupport::TestCase
   context "#after_create for 'issue_updated'" do
     should "should send a notification when configured as a notification" do
       Setting.notified_events = ['issue_updated']
-      assert_difference('ActionMailer::Base.deliveries.size') do
+      assert_difference('ActionMailer::Base.deliveries.size', 2) do
         @issue.init_journal(@user)
         @issue.subject = "A change to the issue"
         assert @issue.save
@@ -46,7 +46,7 @@ class JournalObserverTest < ActiveSupport::TestCase
   context "#after_create for 'issue_note_added'" do
     should "should send a notification when configured as a notification" do
       Setting.notified_events = ['issue_note_added']
-      assert_difference('ActionMailer::Base.deliveries.size') do
+      assert_difference('ActionMailer::Base.deliveries.size', 2) do
         @issue.init_journal(@user, 'This update has a note')
         assert @issue.save
       end
@@ -66,7 +66,7 @@ class JournalObserverTest < ActiveSupport::TestCase
   context "#after_create for 'issue_status_updated'" do
     should "should send a notification when configured as a notification" do
       Setting.notified_events = ['issue_status_updated']
-      assert_difference('ActionMailer::Base.deliveries.size') do
+      assert_difference('ActionMailer::Base.deliveries.size', 2) do
         @issue.init_journal(@user)
         @issue.status = IssueStatus.generate!
         assert @issue.save
@@ -89,7 +89,7 @@ class JournalObserverTest < ActiveSupport::TestCase
   context "#after_create for 'issue_priority_updated'" do
     should "should send a notification when configured as a notification" do
       Setting.notified_events = ['issue_priority_updated']
-      assert_difference('ActionMailer::Base.deliveries.size') do
+      assert_difference('ActionMailer::Base.deliveries.size', 2) do
         @issue.init_journal(@user)
         @issue.priority = IssuePriority.generate!
         assert @issue.save
diff --git a/test/unit/journal_test.rb b/test/unit/journal_test.rb
index 71831982..c59b5a9f 100644
--- a/test/unit/journal_test.rb
+++ b/test/unit/journal_test.rb
@@ -45,7 +45,7 @@ class JournalTest < ActiveSupport::TestCase
     assert_equal 0, ActionMailer::Base.deliveries.size
     issue.reload
     issue.update_attribute(:subject, "New subject to trigger automatic journal entry")
-    assert_equal 1, ActionMailer::Base.deliveries.size
+    assert_equal 2, ActionMailer::Base.deliveries.size
   end
 
   def test_create_should_not_send_email_notification_if_told_not_to
diff --git a/test/unit/lib/redmine/hook_test.rb b/test/unit/lib/redmine/hook_test.rb
index ef385c1f..bdc8ca68 100644
--- a/test/unit/lib/redmine/hook_test.rb
+++ b/test/unit/lib/redmine/hook_test.rb
@@ -144,14 +144,14 @@ class Redmine::Hook::ManagerTest < ActiveSupport::TestCase
     issue = Issue.find(1)
 
     ActionMailer::Base.deliveries.clear
-    Mailer.deliver_issue_add(issue)
+    Mailer.deliver_issue_add(issue, 'jsmith@somenet.foo')
     mail = ActionMailer::Base.deliveries.last
 
     @hook_module.add_listener(TestLinkToHook)
     hook_helper.call_hook(:view_layouts_base_html_head)
 
     ActionMailer::Base.deliveries.clear
-    Mailer.deliver_issue_add(issue)
+    Mailer.deliver_issue_add(issue, 'jsmith@somenet.foo')
     mail2 = ActionMailer::Base.deliveries.last
 
     assert_equal mail.body, mail2.body
diff --git a/test/unit/mail_handler_test.rb b/test/unit/mail_handler_test.rb
index 2dfed204..a4e68542 100644
--- a/test/unit/mail_handler_test.rb
+++ b/test/unit/mail_handler_test.rb
@@ -333,7 +333,7 @@ class MailHandlerTest < ActiveSupport::TestCase
     ActionMailer::Base.deliveries.clear
     journal = submit_email('ticket_reply.eml')
     assert journal.is_a?(Journal)
-    assert_equal 1, ActionMailer::Base.deliveries.size
+    assert_equal 3, ActionMailer::Base.deliveries.size
   end
 
   def test_add_issue_note_should_not_set_defaults
diff --git a/test/unit/mailer_test.rb b/test/unit/mailer_test.rb
index 48a3e970..fbdc7c92 100644
--- a/test/unit/mailer_test.rb
+++ b/test/unit/mailer_test.rb
@@ -19,6 +19,7 @@ class MailerTest < ActiveSupport::TestCase
   fixtures :all
 
   def setup
+    User.current = nil # Clear current user in case of tests setting it and leaking data
     ActionMailer::Base.deliveries.clear
     Setting.host_name = 'mydomain.foo'
     Setting.protocol = 'http'
@@ -30,8 +31,8 @@ class MailerTest < ActiveSupport::TestCase
     Setting.protocol = 'https'
 
     journal = Journal.find(2)
-    assert Mailer.deliver_issue_edit(journal)
-
+    assert Mailer.deliver_issue_edit(journal,'dlopper@somenet.foo')
+    
     mail = ActionMailer::Base.deliveries.last
     assert_kind_of TMail::Mail, mail
 
@@ -52,8 +53,8 @@ class MailerTest < ActiveSupport::TestCase
     Redmine::Utils.relative_url_root = '/rdm'
 
     journal = Journal.find(2)
-    assert Mailer.deliver_issue_edit(journal)
-
+    assert Mailer.deliver_issue_edit(journal,'dlopper@somenet.foo')
+    
     mail = ActionMailer::Base.deliveries.last
     assert_kind_of TMail::Mail, mail
 
@@ -77,8 +78,8 @@ class MailerTest < ActiveSupport::TestCase
     Redmine::Utils.relative_url_root = nil
 
     journal = Journal.find(2)
-    assert Mailer.deliver_issue_edit(journal)
-
+    assert Mailer.deliver_issue_edit(journal,'dlopper@somenet.foo')
+    
     mail = ActionMailer::Base.deliveries.last
     assert_kind_of TMail::Mail, mail
 
@@ -97,7 +98,7 @@ class MailerTest < ActiveSupport::TestCase
 
   def test_email_headers
     issue = Issue.find(1)
-    Mailer.deliver_issue_add(issue)
+    Mailer.deliver_issue_add(issue,'dlopper@somenet.foo')
     mail = ActionMailer::Base.deliveries.last
     assert_not_nil mail
     assert_equal 'bulk', mail.header_string('Precedence')
@@ -107,7 +108,7 @@ class MailerTest < ActiveSupport::TestCase
   def test_plain_text_mail
     Setting.plain_text_mail = 1
     journal = Journal.find(2)
-    Mailer.deliver_issue_edit(journal)
+    Mailer.deliver_issue_edit(journal,'dlopper@somenet.foo')
     mail = ActionMailer::Base.deliveries.last
     assert_equal "text/plain", mail.content_type
     assert_equal 0, mail.parts.size
@@ -117,7 +118,7 @@ class MailerTest < ActiveSupport::TestCase
   def test_html_mail
     Setting.plain_text_mail = 0
     journal = Journal.find(2)
-    Mailer.deliver_issue_edit(journal)
+    Mailer.deliver_issue_edit(journal,'dlopper@somenet.foo')
     mail = ActionMailer::Base.deliveries.last
     assert_equal 2, mail.parts.size
     assert mail.encoded.include?('href')
@@ -141,21 +142,21 @@ class MailerTest < ActiveSupport::TestCase
     user.pref[:no_self_notified] = false
     user.pref.save
     User.current = user
-    Mailer.deliver_news_added(news.reload)
-    assert_equal 1, last_email.bcc.size
+    Mailer.deliver_news_added(news.reload, user.mail)
+    assert_equal 1, last_email.to.size
 
     # nobody to notify
     user.pref[:no_self_notified] = true
     user.pref.save
     User.current = user
     ActionMailer::Base.deliveries.clear
-    Mailer.deliver_news_added(news.reload)
+    Mailer.deliver_news_added(news.reload, user.mail)
     assert ActionMailer::Base.deliveries.empty?
   end
 
   def test_issue_add_message_id
     issue = Issue.find(1)
-    Mailer.deliver_issue_add(issue)
+    Mailer.deliver_issue_add(issue, 'dlopper@somenet.foo')
     mail = ActionMailer::Base.deliveries.last
     assert_not_nil mail
     assert_equal Mailer.message_id_for(issue), mail.message_id
@@ -164,7 +165,7 @@ class MailerTest < ActiveSupport::TestCase
 
   def test_issue_edit_message_id
     journal = Journal.find(1)
-    Mailer.deliver_issue_edit(journal)
+    Mailer.deliver_issue_edit(journal, "jsmith@somenet.foo")
     mail = ActionMailer::Base.deliveries.last
     assert_not_nil mail
     assert_equal Mailer.message_id_for(journal), mail.message_id
@@ -173,7 +174,7 @@ class MailerTest < ActiveSupport::TestCase
 
   def test_message_posted_message_id
     message = Message.find(1)
-    Mailer.deliver_message_posted(message)
+    Mailer.deliver_message_posted(message, "jsmith@somenet.foo")
     mail = ActionMailer::Base.deliveries.last
     assert_not_nil mail
     assert_equal Mailer.message_id_for(message), mail.message_id
@@ -186,7 +187,7 @@ class MailerTest < ActiveSupport::TestCase
 
   def test_reply_posted_message_id
     message = Message.find(3)
-    Mailer.deliver_message_posted(message)
+    Mailer.deliver_message_posted(message, "jsmith@somenet.foo")
     mail = ActionMailer::Base.deliveries.last
     assert_not_nil mail
     assert_equal Mailer.message_id_for(message), mail.message_id
@@ -204,36 +205,10 @@ class MailerTest < ActiveSupport::TestCase
       @issue = Issue.find(1)
     end
 
-    should "notify project members" do
-      assert Mailer.deliver_issue_add(@issue)
-      assert last_email.bcc.include?('dlopper@somenet.foo')
-    end
-
-    should "not notify project members that are not allow to view the issue" do
-      Role.find(2).remove_permission!(:view_issues)
-      assert Mailer.deliver_issue_add(@issue)
-      assert !last_email.bcc.include?('dlopper@somenet.foo')
-    end
-
-    should "notify issue watchers" do
-      user = User.find(9)
-      # minimal email notification options
-      user.pref[:no_self_notified] = '1'
-      user.pref.save
-      user.mail_notification = false
-      user.save
-
-      Watcher.create!(:watchable => @issue, :user => user)
-      assert Mailer.deliver_issue_add(@issue)
-      assert last_email.bcc.include?(user.mail)
-    end
-
-    should "not notify watchers not allowed to view the issue" do
-      user = User.find(9)
-      Watcher.create!(:watchable => @issue, :user => user)
-      Role.non_member.remove_permission!(:view_issues)
-      assert Mailer.deliver_issue_add(@issue)
-      assert !last_email.bcc.include?(user.mail)
+    should "send one email per recipient" do
+      assert Mailer.deliver_issue_add(@issue, 'dlopper@somenet.foo')
+      assert_equal 1, ActionMailer::Base.deliveries.length
+      assert_equal ['dlopper@somenet.foo'], last_email.to
     end
   end
 
@@ -242,7 +217,7 @@ class MailerTest < ActiveSupport::TestCase
     issue = Issue.find(1)
     valid_languages.each do |lang|
       Setting.default_language = lang.to_s
-      assert Mailer.deliver_issue_add(issue)
+      assert Mailer.deliver_issue_add(issue, 'dlopper@somenet.foo')
     end
   end
 
@@ -250,7 +225,7 @@ class MailerTest < ActiveSupport::TestCase
     journal = Journal.find(1)
     valid_languages.each do |lang|
       Setting.default_language = lang.to_s
-      assert Mailer.deliver_issue_edit(journal)
+      assert Mailer.deliver_issue_edit(journal, "jsmith@somenet.foo")
     end
   end
 
@@ -258,7 +233,7 @@ class MailerTest < ActiveSupport::TestCase
     document = Document.find(1)
     valid_languages.each do |lang|
       Setting.default_language = lang.to_s
-      assert Mailer.deliver_document_added(document)
+      assert Mailer.deliver_document_added(document, "jsmith@somenet.foo")
     end
   end
 
@@ -266,35 +241,27 @@ class MailerTest < ActiveSupport::TestCase
     attachements = [ Attachment.find_by_container_type('Document') ]
     valid_languages.each do |lang|
       Setting.default_language = lang.to_s
-      assert Mailer.deliver_attachments_added(attachements)
+      assert Mailer.deliver_attachments_added(attachements, "jsmith@somenet.foo")
     end
   end
 
   def test_version_file_added
     attachements = [ Attachment.find_by_container_type('Version') ]
-    assert Mailer.deliver_attachments_added(attachements)
-    assert_not_nil last_email.bcc
-    assert last_email.bcc.any?
-    assert_select_email do
-      assert_select "a[href=?]", "http://mydomain.foo/projects/ecookbook/files"
-    end
+    assert Mailer.deliver_attachments_added(attachements, "jsmith@somenet.foo")
+    assert_equal ["jsmith@somenet.foo"], last_email.to
   end
 
   def test_project_file_added
     attachements = [ Attachment.find_by_container_type('Project') ]
-    assert Mailer.deliver_attachments_added(attachements)
-    assert_not_nil last_email.bcc
-    assert last_email.bcc.any?
-    assert_select_email do
-      assert_select "a[href=?]", "http://mydomain.foo/projects/ecookbook/files"
-    end
+    assert Mailer.deliver_attachments_added(attachements, "jsmith@somenet.foo")
+    assert_equal ["jsmith@somenet.foo"], last_email.to
   end
 
   def test_news_added
     news = News.find(:first)
     valid_languages.each do |lang|
       Setting.default_language = lang.to_s
-      assert Mailer.deliver_news_added(news)
+      assert Mailer.deliver_news_added(news, "jsmith@somenet.foo")
     end
   end
 
@@ -308,11 +275,9 @@ class MailerTest < ActiveSupport::TestCase
 
   def test_message_posted
     message = Message.find(:first)
-    recipients = ([message.root] + message.root.children).collect {|m| m.author.mail if m.author}
-    recipients = recipients.compact.uniq
     valid_languages.each do |lang|
       Setting.default_language = lang.to_s
-      assert Mailer.deliver_message_posted(message)
+      assert Mailer.deliver_message_posted(message, "jsmith@somenet.foo")
     end
   end
 
@@ -321,7 +286,7 @@ class MailerTest < ActiveSupport::TestCase
     valid_languages.each do |lang|
       Setting.default_language = lang.to_s
       assert_difference 'ActionMailer::Base.deliveries.size' do
-        assert Mailer.deliver_wiki_content_added(content)
+        assert Mailer.deliver_wiki_content_added(content, "jsmith@somenet.foo")
       end
     end
   end
@@ -331,7 +296,7 @@ class MailerTest < ActiveSupport::TestCase
     valid_languages.each do |lang|
       Setting.default_language = lang.to_s
       assert_difference 'ActionMailer::Base.deliveries.size' do
-        assert Mailer.deliver_wiki_content_updated(content)
+        assert Mailer.deliver_wiki_content_updated(content, "jsmith@somenet.foo")
       end
     end
   end
@@ -381,18 +346,18 @@ class MailerTest < ActiveSupport::TestCase
     Mailer.reminders(:days => 42)
     assert_equal 1, ActionMailer::Base.deliveries.size
     mail = ActionMailer::Base.deliveries.last
-    assert mail.bcc.include?('dlopper@somenet.foo')
+    assert mail.to.include?('dlopper@somenet.foo')
     assert mail.body.include?('Bug #3: Error 281 when updating a recipe')
     assert_equal '1 issue(s) due in the next 42 days', mail.subject
   end
 
   def test_reminders_for_users
     Mailer.reminders(:days => 42, :users => ['5'])
-    assert_equal 0, ActionMailer::Base.deliveries.size # No mail for dlopper
+    assert_equal 0, ActionMailer::Base.deliveries.size
     Mailer.reminders(:days => 42, :users => ['3'])
-    assert_equal 1, ActionMailer::Base.deliveries.size # No mail for dlopper
+    assert_equal 1, ActionMailer::Base.deliveries.size
     mail = ActionMailer::Base.deliveries.last
-    assert mail.bcc.include?('dlopper@somenet.foo')
+    assert mail.to.include?('dlopper@somenet.foo')
     assert mail.body.include?('Bug #3: Error 281 when updating a recipe')
   end
 
diff --git a/test/unit/message_test.rb b/test/unit/message_test.rb
index 95dd0193..85037d3b 100644
--- a/test/unit/message_test.rb
+++ b/test/unit/message_test.rb
@@ -14,7 +14,7 @@
 require File.expand_path('../../test_helper', __FILE__)
 
 class MessageTest < ActiveSupport::TestCase
-  fixtures :projects, :roles, :members, :member_roles, :boards, :messages, :users, :watchers
+  fixtures :all
 
   def setup
     Setting.notified_events = ['message_posted']
@@ -142,7 +142,7 @@ class MessageTest < ActiveSupport::TestCase
   end
 
   test "email notifications for creating a message" do
-    assert_difference("ActionMailer::Base.deliveries.count") do
+    assert_difference("ActionMailer::Base.deliveries.count", 3) do
       message = Message.new(:board => @board, :subject => 'Test message', :content => 'Test message content', :author => @user)
       assert message.save
     end
diff --git a/test/unit/news_test.rb b/test/unit/news_test.rb
index c94058ef..c027f155 100644
--- a/test/unit/news_test.rb
+++ b/test/unit/news_test.rb
@@ -30,7 +30,7 @@ class NewsTest < ActiveSupport::TestCase
     news = Project.find(:first).news.new(valid_news)
 
     assert news.save
-    assert_equal 1, ActionMailer::Base.deliveries.size
+    assert_equal 2, ActionMailer::Base.deliveries.size
   end
 
   def test_should_include_news_for_projects_with_news_enabled
diff --git a/test/unit/repository_test.rb b/test/unit/repository_test.rb
index 0a5f1eca..a44814d4 100644
--- a/test/unit/repository_test.rb
+++ b/test/unit/repository_test.rb
@@ -96,9 +96,9 @@ class RepositoryTest < ActiveSupport::TestCase
     journal = fixed_issue.journals.last
     assert_equal User.find_by_login('dlopper'), journal.user
     assert_equal 'Applied in changeset r2.', journal.notes
-
-    # 2 email notifications
-    assert_equal 2, ActionMailer::Base.deliveries.size
+    
+    # 2 email notifications to 5 users
+    assert_equal 5, ActionMailer::Base.deliveries.size
     mail = ActionMailer::Base.deliveries.first
     assert_kind_of TMail::Mail, mail
     assert mail.subject.starts_with?("[#{fixed_issue.project.name} - #{fixed_issue.tracker.name} ##{fixed_issue.id}]")
diff --git a/test/unit/wiki_content_test.rb b/test/unit/wiki_content_test.rb
index 33cbedba..6a3f609e 100644
--- a/test/unit/wiki_content_test.rb
+++ b/test/unit/wiki_content_test.rb
@@ -43,8 +43,8 @@ class WikiContentTest < ActiveSupport::TestCase
     page = WikiPage.new(:wiki => @wiki, :title => "A new page")
     page.content = WikiContent.new(:text => "Content text", :author => User.find(1), :comments => "My comment")
     assert page.save
-
-    assert_equal 1, ActionMailer::Base.deliveries.size
+    
+    assert_equal 2, ActionMailer::Base.deliveries.size
   end
 
   def test_update
@@ -63,8 +63,8 @@ class WikiContentTest < ActiveSupport::TestCase
     content = @page.content
     content.text = "My new content"
     assert content.save
-
-    assert_equal 1, ActionMailer::Base.deliveries.size
+    
+    assert_equal 2, ActionMailer::Base.deliveries.size
   end
 
   def test_fetch_history