From c8b722456c5d73e0a85c2073cd3fd7cd5c35a29c Mon Sep 17 00:00:00 2001
From: Jean-Philippe Lang <jp_lang@yahoo.fr>
Date: Mon, 5 Nov 2012 15:49:07 +0000
Subject: [PATCH] Fixed that watchers receive notifications for private
 comments without permission (#12286).

git-svn-id: svn+ssh://rubyforge.org/var/svn/redmine/trunk@10789 e93f8b46-1217-0410-a6f0-8f06a7374b81
---
 app/models/journal.rb                             |  8 ++++++++
 app/models/mailer.rb                              |  2 +-
 .../acts_as_watchable/lib/acts_as_watchable.rb    | 13 ++++++++-----
 test/unit/mailer_test.rb                          | 15 +++++++++++++++
 4 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/app/models/journal.rb b/app/models/journal.rb
index 895b8537d..560ccf3b8 100644
--- a/app/models/journal.rb
+++ b/app/models/journal.rb
@@ -100,6 +100,14 @@ class Journal < ActiveRecord::Base
     notified.map(&:mail)
   end
 
+  def watcher_recipients
+    notified = journalized.notified_watchers
+    if private_notes?
+      notified = notified.select {|user| user.allowed_to?(:view_private_notes, journalized.project)}
+    end
+    notified.map(&:mail)
+  end
+
   private
 
   def split_private_notes
diff --git a/app/models/mailer.rb b/app/models/mailer.rb
index 91b58dda2..9c19438a7 100644
--- a/app/models/mailer.rb
+++ b/app/models/mailer.rb
@@ -64,7 +64,7 @@ class Mailer < ActionMailer::Base
     @author = journal.user
     recipients = journal.recipients
     # Watchers in cc
-    cc = issue.watcher_recipients - recipients
+    cc = journal.watcher_recipients - recipients
     s = "[#{issue.project.name} - #{issue.tracker.name} ##{issue.id}] "
     s << "(#{issue.status.name}) " if journal.new_value_for('status_id')
     s << issue.subject
diff --git a/lib/plugins/acts_as_watchable/lib/acts_as_watchable.rb b/lib/plugins/acts_as_watchable/lib/acts_as_watchable.rb
index 1f8877584..c284af38d 100644
--- a/lib/plugins/acts_as_watchable/lib/acts_as_watchable.rb
+++ b/lib/plugins/acts_as_watchable/lib/acts_as_watchable.rb
@@ -67,15 +67,18 @@ module Redmine
           !!(user && self.watcher_user_ids.detect {|uid| uid == user.id })
         end
 
-        # Returns an array of watchers' email addresses
-        def watcher_recipients
+        def notified_watchers
           notified = watcher_users.active
-          notified.reject! {|user| user.mail_notification == 'none'}
-
+          notified.reject! {|user| user.mail.blank? || user.mail_notification == 'none'}
           if respond_to?(:visible?)
             notified.reject! {|user| !visible?(user)}
           end
-          notified.collect(&:mail).compact
+          notified
+        end
+
+        # Returns an array of watchers' email addresses
+        def watcher_recipients
+          notified_watchers.collect(&:mail)
         end
 
         module ClassMethods; end
diff --git a/test/unit/mailer_test.rb b/test/unit/mailer_test.rb
index 8d65cc5b7..a92f738a7 100644
--- a/test/unit/mailer_test.rb
+++ b/test/unit/mailer_test.rb
@@ -350,6 +350,21 @@ class MailerTest < ActiveSupport::TestCase
     assert_equal %w(jsmith@somenet.foo), ActionMailer::Base.deliveries.last.bcc.sort
   end
 
+  def test_issue_edit_should_send_private_notes_to_watchers_with_permission_only
+    Issue.find(1).set_watcher(User.find_by_login('someone'))
+    journal = Journal.find(1)
+    journal.private_notes = true
+    journal.save!
+
+    Role.non_member.add_permission! :view_private_notes
+    Mailer.issue_edit(journal).deliver
+    assert_include 'someone@foo.bar', ActionMailer::Base.deliveries.last.bcc.sort
+
+    Role.non_member.remove_permission! :view_private_notes
+    Mailer.issue_edit(journal).deliver
+    assert_not_include 'someone@foo.bar', ActionMailer::Base.deliveries.last.bcc.sort
+  end
+
   def test_document_added
     document = Document.find(1)
     valid_languages.each do |lang|