Change Project#notified_users to check for the 'all' notification option. #6541

The previous mail_notification? check would always pass since the
notifications where converted to strings and strings are always true.

Also changed Project#recipients to use #notified_users instead of duplicated
code.

Based on contribution by Felix Schäfer.

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

diff --git a/app/models/project.rb b/app/models/project.rb
index 0bb67e4207cae24976e1eaf15d2c0097ffd59b78..000efa7c2f01b88bfb6fa1391754e0bbad27a3ee 100644 (file)
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -382,12 +382,13 @@ class Project < ActiveRecord::Base
   
   # Returns the mail adresses of users that should be always notified on project events
   def recipients
-    members.select {|m| m.mail_notification? || m.user.mail_notification == 'all'}.collect {|m| m.user.mail}
+    notified_users.collect {|user| 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}
+    # TODO: User part should be extracted to User#notify_about?
+    members.select {|m| m.mail_notification? || m.user.mail_notification == 'all'}.collect {|m| m.user}
   end
   
   # Returns an array of all custom fields enabled for project issues

diff --git a/test/unit/project_test.rb b/test/unit/project_test.rb
index 9b8809c2bb1ac877f207cf69dd4530daf6aafb91..08b0fb24a8bc7787a95b507959ccebd04ee83521 100644 (file)
--- a/test/unit/project_test.rb
+++ b/test/unit/project_test.rb
@@ -964,4 +964,54 @@ class ProjectTest < ActiveSupport::TestCase
 
     end
   end
+
+  context "#notified_users" do
+    setup do
+      @project = Project.generate!
+      @role = Role.generate!
+      
+      @user_with_membership_notification = User.generate!(:mail_notification => 'selected')
+      Member.generate!(:project => @project, :roles => [@role], :principal => @user_with_membership_notification, :mail_notification => true)
+
+      @all_events_user = User.generate!(:mail_notification => 'all')
+      Member.generate!(:project => @project, :roles => [@role], :principal => @all_events_user)
+
+      @no_events_user = User.generate!(:mail_notification => 'none')
+      Member.generate!(:project => @project, :roles => [@role], :principal => @no_events_user)
+
+      @only_my_events_user = User.generate!(:mail_notification => 'only_my_events')
+      Member.generate!(:project => @project, :roles => [@role], :principal => @only_my_events_user)
+
+      @only_assigned_user = User.generate!(:mail_notification => 'only_assigned')
+      Member.generate!(:project => @project, :roles => [@role], :principal => @only_assigned_user)
+
+      @only_owned_user = User.generate!(:mail_notification => 'only_owner')
+      Member.generate!(:project => @project, :roles => [@role], :principal => @only_owned_user)
+    end
+    
+    should "include members with a mail notification" do
+      assert @project.notified_users.include?(@user_with_membership_notification)
+    end
+    
+    should "include users with the 'all' notification option" do
+      assert @project.notified_users.include?(@all_events_user)
+    end
+    
+    should "not include users with the 'none' notification option" do
+      assert !@project.notified_users.include?(@no_events_user)
+    end
+    
+    should "not include users with the 'only_my_events' notification option" do
+      assert !@project.notified_users.include?(@only_my_events_user)
+    end
+    
+    should "not include users with the 'only_assigned' notification option" do
+      assert !@project.notified_users.include?(@only_assigned_user)
+    end
+    
+    should "not include users with the 'only_owner' notification option" do
+      assert !@project.notified_users.include?(@only_owned_user)
+    end
+  end
+  
 end