From: Eric Davis <edavis@littlestreamsoftware.com>
Date: Tue, 28 Sep 2010 18:22:10 +0000 (+0000)
Subject: Added User#notify_about? to check when a user should be notified about an event
X-Git-Tag: 1.1.0~317
X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=c059300d99aa439c83372a950087f082c7049018;p=redmine.git

Added User#notify_about? to check when a user should be notified about an event

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

diff --git a/app/models/issue.rb b/app/models/issue.rb
index b72d7b4ad..44b4ba80b 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -415,9 +415,10 @@ class Issue < ActiveRecord::Base
   # Returns the mail adresses of users that should be notified
   def recipients
     notified = project.notified_users
-    # Author and assignee are always notified unless they have been locked
-    notified << author if author && author.active?
-    notified << assigned_to if assigned_to && assigned_to.active?
+    # Author and assignee are always notified unless they have been
+    # locked or don't want to be notified
+    notified << author if author && author.active? && author.notify_about?(self)
+    notified << assigned_to if assigned_to && assigned_to.active? && assigned_to.notify_about?(self)
     notified.uniq!
     # Remove users that can not view the issue
     notified.reject! {|user| !visible?(user)}
diff --git a/app/models/user.rb b/app/models/user.rb
index 9fe5ff2ab..281e71281 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -368,6 +368,41 @@ class User < Principal
     allowed_to?(action, nil, options.reverse_merge(:global => true))
   end
   
+  # Utility method to help check if a user should be notified about an
+  # event.
+  #
+  # TODO: only supports Issue events currently
+  def notify_about?(object)
+    case mail_notification.to_sym
+    when :all
+      true
+    when :selected
+      # Handled by the Project
+    when :none
+      false
+    when :only_my_events
+      if object.is_a?(Issue) && (object.author == self || object.assigned_to == self)
+        true
+      else
+        false
+      end
+    when :only_assigned
+      if object.is_a?(Issue) && object.assigned_to == self
+        true
+      else
+        false
+      end
+    when :only_owner
+      if object.is_a?(Issue) && object.author == self
+        true
+      else
+        false
+      end
+    else
+      false
+    end
+  end
+  
   def self.current=(user)
     @current_user = user
   end
diff --git a/test/exemplars/user_exemplar.rb b/test/exemplars/user_exemplar.rb
index 7c4af70b8..def8dc423 100644
--- a/test/exemplars/user_exemplar.rb
+++ b/test/exemplars/user_exemplar.rb
@@ -3,7 +3,7 @@ class User < Principal
   generator_for :mail, :method => :next_email
   generator_for :firstname, :method => :next_firstname
   generator_for :lastname, :method => :next_lastname
-
+  
   def self.next_login
     @gen_login ||= 'user1'
     @gen_login.succ!
diff --git a/test/unit/issue_test.rb b/test/unit/issue_test.rb
index d293cb88b..13a24cd69 100644
--- a/test/unit/issue_test.rb
+++ b/test/unit/issue_test.rb
@@ -742,7 +742,9 @@ class IssueTest < ActiveSupport::TestCase
   context "Issue#recipients" do
     setup do
       @project = Project.find(1)
-      @issue = Issue.generate_for_project!(@project, :assigned_to => User.generate_with_protected!)
+      @author = User.generate_with_protected!
+      @assignee = User.generate_with_protected!
+      @issue = Issue.generate_for_project!(@project, :assigned_to => @assignee, :author => @author)
     end
     
     should "include project recipients" do
@@ -761,5 +763,24 @@ class IssueTest < ActiveSupport::TestCase
       assert @issue.assigned_to, "No assigned_to set for Issue"
       assert @issue.recipients.include?(@issue.assigned_to.mail)
     end
+
+    should "not include users who opt out of all email" do
+      @author.update_attribute(:mail_notification, :none)
+
+      assert !@issue.recipients.include?(@issue.author.mail)
+    end
+
+    should "not include the issue author if they are only notified of assigned issues" do
+      @author.update_attribute(:mail_notification, :only_assigned)
+
+      assert !@issue.recipients.include?(@issue.author.mail)
+    end
+
+    should "not include the assigned user if they are only notified of owned issues" do
+      @assignee.update_attribute(:mail_notification, :only_owner)
+
+      assert !@issue.recipients.include?(@issue.assigned_to.mail)
+    end
+
   end
 end
diff --git a/test/unit/mail_handler_test.rb b/test/unit/mail_handler_test.rb
index e0ff8a2a7..ab366f1e0 100644
--- a/test/unit/mail_handler_test.rb
+++ b/test/unit/mail_handler_test.rb
@@ -152,7 +152,7 @@ class MailHandlerTest < ActiveSupport::TestCase
     assert !issue.new_record?
     issue.reload
     assert issue.watched_by?(User.find_by_mail('dlopper@somenet.foo'))
-    assert_equal 1, issue.watchers.size
+    assert_equal 1, issue.watcher_user_ids.size
   end
   
   def test_add_issue_by_unknown_user
diff --git a/test/unit/user_test.rb b/test/unit/user_test.rb
index 1ecbc5927..07dada4e7 100644
--- a/test/unit/user_test.rb
+++ b/test/unit/user_test.rb
@@ -398,6 +398,72 @@ class UserTest < ActiveSupport::TestCase
     end
   end
   
+  context "User#notify_about?" do
+    context "Issues" do
+      setup do
+        @project = Project.find(1)
+        @author = User.generate_with_protected!
+        @assignee = User.generate_with_protected!
+        @issue = Issue.generate_for_project!(@project, :assigned_to => @assignee, :author => @author)
+      end
+
+      should "be true for a user with :all" do
+        @author.update_attribute(:mail_notification, :all)
+        assert @author.notify_about?(@issue)
+      end
+      
+      should "be false for a user with :none" do
+        @author.update_attribute(:mail_notification, :none)
+        assert ! @author.notify_about?(@issue)
+      end
+      
+      should "be false for a user with :only_my_events and isn't an author, creator, or assignee" do
+        @user = User.generate_with_protected!(:mail_notification => :only_my_events)
+        assert ! @user.notify_about?(@issue)
+      end
+      
+      should "be true for a user with :only_my_events and is the author" do
+        @author.update_attribute(:mail_notification, :only_my_events)
+        assert @author.notify_about?(@issue)
+      end
+      
+      should "be true for a user with :only_my_events and is the assignee" do
+        @assignee.update_attribute(:mail_notification, :only_my_events)
+        assert @assignee.notify_about?(@issue)
+      end
+      
+      should "be true for a user with :only_assigned and is the assignee" do
+        @assignee.update_attribute(:mail_notification, :only_assigned)
+        assert @assignee.notify_about?(@issue)
+      end
+      
+      should "be false for a user with :only_assigned and is not the assignee" do
+        @author.update_attribute(:mail_notification, :only_assigned)
+        assert ! @author.notify_about?(@issue)
+      end
+      
+      should "be true for a user with :only_owner and is the author" do
+        @author.update_attribute(:mail_notification, :only_owner)
+        assert @author.notify_about?(@issue)
+      end
+      
+      should "be false for a user with :only_owner and is not the author" do
+        @assignee.update_attribute(:mail_notification, :only_owner)
+        assert ! @assignee.notify_about?(@issue)
+      end
+      
+      should "be false if the mail_notification is anything else" do
+        @assignee.update_attribute(:mail_notification, :somthing_else)
+        assert ! @assignee.notify_about?(@issue)
+      end
+      
+    end
+
+    context "other events" do
+      should 'be added and tested'
+    end
+  end
+  
   if Object.const_defined?(:OpenID)
     
   def test_setting_identity_url