Do not notify users that are no longer allowed to view an issue (#3589, #4263).

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

4 files changed, 38 insertions, 6 deletions

diff --git a/app/models/issue.rb b/app/models/issue.rb
index d279f3c92..1facf2a56 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -250,13 +250,23 @@ class Issue < ActiveRecord::Base
     blocked? ? statuses.reject {|s| s.is_closed?} : statuses
   end
   
-  # Returns the mail adresses of users that should be notified for the issue
+  # Returns the mail adresses of users that should be notified
   def recipients
-    recipients = project.recipients
+    notified = project.notified_users
     # Author and assignee are always notified unless they have been locked
-    recipients << author.mail if author && author.active?
-    recipients << assigned_to.mail if assigned_to && assigned_to.active?
-    recipients.compact.uniq
+    notified << author if author && author.active?
+    notified << assigned_to if assigned_to && assigned_to.active?
+    notified.uniq!
+    # Remove users that can not view the issue
+    notified.reject! {|user| !visible?(user)}
+    notified.collect(&:mail)
+  end
+  
+  # Returns the mail adresses of watchers that should be notified
+  def watcher_recipients
+    notified = watcher_users
+    notified.reject! {|user| !user.active? || !visible?(user)}
+    notified.collect(&:mail)
   end
   
   # Returns the total number of hours spent on this issue.
diff --git a/app/models/project.rb b/app/models/project.rb
index 8829f04ad..5cc8ab9d0 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -352,6 +352,11 @@ class Project < ActiveRecord::Base
     members.select {|m| m.mail_notification? || m.user.mail_notification?}.collect {|m| m.user.mail}
   end
   
+  # Returns the users that should be notified on project events
+  def notified_users
+    members.select {|m| m.mail_notification? || m.user.mail_notification?}.collect {|m| m.user}
+  end
+  
   # Returns an array of all custom fields enabled for project issues
   # (explictly associated custom fields and custom fields enabled for all projects)
   def all_issue_custom_fields
diff --git a/test/fixtures/issues.yml b/test/fixtures/issues.yml
index 03197458d..a6a5be3a7 100644
--- a/test/fixtures/issues.yml
+++ b/test/fixtures/issues.yml
@@ -185,7 +185,7 @@ issues_012:
   description:
   tracker_id: 1
   assigned_to_id: 
-  author_id: 2
+  author_id: 3
   status_id: 5
   start_date: <%= 1.day.ago.to_date.to_s(:db) %>
   due_date:
diff --git a/test/unit/issue_test.rb b/test/unit/issue_test.rb
index afde6c720..bd37f9880 100644
--- a/test/unit/issue_test.rb
+++ b/test/unit/issue_test.rb
@@ -353,6 +353,23 @@ class IssueTest < ActiveSupport::TestCase
     assert_nil copy.custom_value_for(2)
   end
   
+  def test_recipients_should_not_include_users_that_cannot_view_the_issue
+    issue = Issue.find(12)
+    assert issue.recipients.include?(issue.author.mail)
+    # move the issue to a private project
+    copy  = issue.move_to(Project.find(5), Tracker.find(2), :copy => true)
+    # author is not a member of project anymore
+    assert !copy.recipients.include?(copy.author.mail)
+  end
+
+  def test_watcher_recipients_should_not_include_users_that_cannot_view_the_issue
+    user = User.find(3)
+    issue = Issue.find(9)
+    Watcher.create!(:user => user, :watchable => issue)
+    assert issue.watched_by?(user)
+    assert !issue.watcher_recipients.include?(user.mail)
+  end
+  
   def test_issue_destroy
     Issue.find(1).destroy
     assert_nil Issue.find_by_id(1)