Allow adding user groups as watchers for issues (#4511).

Patch by Yuichi HARADA.


git-svn-id: http://svn.redmine.org/redmine/trunk@19498 e93f8b46-1217-0410-a6f0-8f06a7374b81

7 files changed, 129 insertions, 49 deletions

diff --git a/test/functional/issues_controller_test.rb b/test/functional/issues_controller_test.rb
index 11d93cf21..2dcf06bb1 100644
--- a/test/functional/issues_controller_test.rb
+++ b/test/functional/issues_controller_test.rb
@@ -2443,28 +2443,41 @@ class IssuesControllerTest < Redmine::ControllerTest
 
   def test_show_should_display_watchers
     @request.session[:user_id] = 2
-    Issue.find(1).add_watcher User.find(2)
+    issue = Issue.find(1)
+    issue.add_watcher User.find(2)
+    issue.add_watcher Group.find(10)
     get(:show, :params => {:id => 1})
     assert_select 'div#watchers ul' do
-      assert_select 'li' do
+      assert_select 'li.user-2' do
         assert_select 'a[href="/users/2"]'
         assert_select 'a[class*=delete]'
       end
+      assert_select "li.user-10" do
+        assert_select 'a[href="/users/10"]', false
+        assert_select 'a[class*=delete]'
+      end
     end
   end
 
   def test_show_should_display_watchers_with_gravatars
     @request.session[:user_id] = 2
-    Issue.find(1).add_watcher User.find(2)
+    issue = Issue.find(1)
+    issue.add_watcher User.find(2)
+    issue.add_watcher Group.find(10)
     with_settings :gravatar_enabled => '1' do
       get(:show, :params => {:id => 1})
     end
     assert_select 'div#watchers ul' do
-      assert_select 'li' do
-        assert_select 'img.gravatar'
+      assert_select 'li.user-2' do
+        assert_select 'img.gravatar[title=?]', 'John Smith'
         assert_select 'a[href="/users/2"]'
         assert_select 'a[class*=delete]'
       end
+      assert_select "li.user-10" do
+        assert_select 'img.gravatar[title=?]', 'A Team'
+        assert_select 'a[href="/users/10"]', false
+        assert_select 'a[class*=delete]'
+      end
     end
   end
 
@@ -3995,7 +4008,7 @@ class IssuesControllerTest < Redmine::ControllerTest
     ActionMailer::Base.deliveries.clear
 
     with_settings :notified_events => %w(issue_added) do
-      assert_difference 'Watcher.count', 2 do
+      assert_difference 'Watcher.count', 3 do
         post(
           :create,
           :params => {
@@ -4005,7 +4018,7 @@ class IssuesControllerTest < Redmine::ControllerTest
               :subject => 'This is a new issue with watchers',
               :description => 'This is the description',
               :priority_id => 5,
-              :watcher_user_ids => ['2', '3']
+              :watcher_user_ids => ['2', '3', '10']
             }
           }
         )
@@ -4016,12 +4029,15 @@ class IssuesControllerTest < Redmine::ControllerTest
     assert_redirected_to :controller => 'issues', :action => 'show', :id => issue
 
     # Watchers added
-    assert_equal [2, 3], issue.watcher_user_ids.sort
+    assert_equal [2, 3, 10], issue.watcher_user_ids.sort
     assert issue.watched_by?(User.find(3))
+    assert issue.watched_by?(Group.find(10))
     # Watchers notified
-    mail = ActionMailer::Base.deliveries.last
-    assert_not_nil mail
+    assert_equal 3, ActionMailer::Base.deliveries.size
+    mail = ActionMailer::Base.deliveries[1]
     assert [mail.bcc, mail.cc].flatten.include?(User.find(3).mail)
+    mail = ActionMailer::Base.deliveries[2]
+    assert [mail.bcc, mail.cc].flatten.include?(User.find(8).mail)
   end
 
   def test_post_create_subissue
@@ -4740,8 +4756,10 @@ class IssuesControllerTest < Redmine::ControllerTest
 
   def test_new_as_copy_should_preserve_watchers
     @request.session[:user_id] = 2
+    issue = Issue.find(1)
     user = User.generate!
-    Watcher.create!(:watchable => Issue.find(1), :user => user)
+    Watcher.create!(:watchable => issue, :user => user)
+    Watcher.create!(:watchable => issue, :user => Group.find(10))
     get(
       :new,
       :params => {
@@ -4749,8 +4767,9 @@ class IssuesControllerTest < Redmine::ControllerTest
         :copy_from => 1
       }
     )
-    assert_select 'input[type=checkbox][name=?][checked=checked]', 'issue[watcher_user_ids][]', 1
+    assert_select 'input[type=checkbox][name=?][checked=checked]', 'issue[watcher_user_ids][]', 2
     assert_select 'input[type=checkbox][name=?][checked=checked][value=?]', 'issue[watcher_user_ids][]', user.id.to_s
+    assert_select 'input[type=checkbox][name=?][checked=checked][value=?]', 'issue[watcher_user_ids][]', '10'
     assert_select 'input[type=hidden][name=?][value=?]', 'issue[watcher_user_ids][]', '', 1
   end
 
@@ -5174,13 +5193,13 @@ class IssuesControllerTest < Redmine::ControllerTest
           :copy_from => copied.id,
           :issue => {
             :subject => 'Copy cleared watchers',
-            :watcher_user_ids => ['', '3']
+            :watcher_user_ids => ['', '3', '10']
           }
         }
       )
     end
     issue = Issue.order('id DESC').first
-    assert_equal [3], issue.watcher_user_ids
+    assert_equal [3, 10], issue.watcher_user_ids
   end
 
   def test_create_as_copy_without_watcher_user_ids_should_not_copy_watchers
@@ -7329,7 +7348,9 @@ class IssuesControllerTest < Redmine::ControllerTest
   end
 
   test "issue bulk copy copy watcher" do
-    Watcher.create!(:watchable => Issue.find(1), :user => User.find(3))
+    issue = Issue.find(1)
+    Watcher.create!(:watchable => issue, :user => User.find(3))
+    Watcher.create!(:watchable => issue, :user => Group.find(10))
     @request.session[:user_id] = 2
     assert_difference 'Issue.count' do
       post(
@@ -7345,7 +7366,8 @@ class IssuesControllerTest < Redmine::ControllerTest
       )
     end
     copy = Issue.order(:id => :desc).first
-    assert_equal 1, copy.watchers.count
+    assert_equal 2, copy.watchers.count
+    assert_equal [3, 10], copy.watcher_user_ids
   end
 
   def test_bulk_copy_should_not_copy_selected_subtasks_twice
diff --git a/test/functional/queries_controller_test.rb b/test/functional/queries_controller_test.rb
index 39e4a20b7..fe5501e32 100644
--- a/test/functional/queries_controller_test.rb
+++ b/test/functional/queries_controller_test.rb
@@ -834,7 +834,7 @@ class QueriesControllerTest < Redmine::ControllerTest
     assert_equal [["<< me >>", "me"]], json
   end
 
-  def test_watcher_filter_with_permission_should_show_members
+  def test_watcher_filter_with_permission_should_show_members_and_groups
     # This user has view_issue_watchers permission
     @request.session[:user_id] = 1
 
@@ -847,10 +847,11 @@ class QueriesControllerTest < Redmine::ControllerTest
     assert_equal 'application/json', response.media_type
     json = ActiveSupport::JSON.decode(response.body)
 
-    assert_equal 6, json.count
+    assert_equal 7, json.count
     # "me" value should not be grouped
     assert_include ["<< me >>", "me"], json
     assert_include ["Dave Lopper", "3", "active"], json
     assert_include ["Dave2 Lopper2", "5", "locked"], json
+    assert_include ["A Team", "10", "active"], json
   end
 end
diff --git a/test/functional/watchers_controller_test.rb b/test/functional/watchers_controller_test.rb
index 8500d2359..280a4d859 100644
--- a/test/functional/watchers_controller_test.rb
+++ b/test/functional/watchers_controller_test.rb
@@ -189,6 +189,19 @@ class WatchersControllerTest < Redmine::ControllerTest
     assert Issue.find(2).watched_by?(User.find(4))
   end
 
+  def test_create_group_as_html
+    @request.session[:user_id] = 2
+    assert_difference('Watcher.count') do
+      post :create, :params => {
+        :object_type => 'issue', :object_id => '2',
+        :watcher => {:user_id => '10'}
+      }
+      assert_response :success
+      assert_include 'Watcher added', response.body
+    end
+    assert Issue.find(2).watched_by?(Group.find(10))
+  end
+
   def test_create
     @request.session[:user_id] = 2
     assert_difference('Watcher.count') do
@@ -205,45 +218,56 @@ class WatchersControllerTest < Redmine::ControllerTest
 
   def test_create_with_mutiple_users
     @request.session[:user_id] = 2
-    assert_difference('Watcher.count', 2) do
+    assert_difference('Watcher.count', 3) do
       post :create, :params => {
         :object_type => 'issue', :object_id => '2',
-        :watcher => {:user_ids => ['4', '7']}
+        :watcher => {:user_ids => ['4', '7', '10']}
       }, :xhr => true
       assert_response :success
       assert_match /watchers/, response.body
       assert_match /ajax-modal/, response.body
     end
-    assert Issue.find(2).watched_by?(User.find(4))
-    assert Issue.find(2).watched_by?(User.find(7))
+    issue = Issue.find(2)
+    assert issue.watched_by?(User.find(4))
+    assert issue.watched_by?(User.find(7))
+    assert issue.watched_by?(Group.find(10))
   end
 
   def test_create_with_mutiple_objects
     @request.session[:user_id] = 2
-    assert_difference('Watcher.count', 4) do
+    assert_difference('Watcher.count', 6) do
       post :create, :params => {
         :object_type => 'issue', :object_id => ['1', '2'],
-        :watcher => {:user_ids => ['4', '7']}
+        :watcher => {:user_ids => ['4', '7', '10']}
       }, :xhr => true
       assert_response :success
       assert_match /watchers/, response.body
       assert_match /ajax-modal/, response.body
     end
-    assert Issue.find(1).watched_by?(User.find(4))
-    assert Issue.find(2).watched_by?(User.find(4))
-    assert Issue.find(1).watched_by?(User.find(7))
-    assert Issue.find(2).watched_by?(User.find(7))
+    issue1 = Issue.find(1)
+    issue2 = Issue.find(2)
+    user4 = User.find(4)
+    user7 = User.find(7)
+    group10 = Group.find(10)
+    assert issue1.watched_by?(user4)
+    assert issue2.watched_by?(user4)
+    assert issue1.watched_by?(user7)
+    assert issue2.watched_by?(user7)
+    assert issue1.watched_by?(group10)
+    assert issue2.watched_by?(group10)
   end
 
   def test_autocomplete_on_watchable_creation
     @request.session[:user_id] = 2
+    group = Group.generate!(:name => 'Group Minimum')
     get :autocomplete_for_user, :params => {:q => 'mi', :project_id => 'ecookbook'}, :xhr => true
     assert_response :success
-    assert_select 'input', :count => 4
+    assert_select 'input', :count => 5
     assert_select 'input[name=?][value="1"]', 'watcher[user_ids][]'
     assert_select 'input[name=?][value="2"]', 'watcher[user_ids][]'
     assert_select 'input[name=?][value="8"]', 'watcher[user_ids][]'
     assert_select 'input[name=?][value="9"]', 'watcher[user_ids][]'
+    assert_select %(input[name=?][value="#{group.id}"]), 'watcher[user_ids][]'
   end
 
   def test_search_non_member_on_create
@@ -342,6 +366,23 @@ class WatchersControllerTest < Redmine::ControllerTest
     assert !Issue.find(2).watched_by?(User.find(3))
   end
 
+  def test_destroy_group_as_html
+    @request.session[:user_id] = 2
+    issue = Issue.find(2)
+    group = Group.find(10)
+    issue.add_watcher(group)
+    assert issue.watched_by?(group)
+    assert_difference('Watcher.count', -1) do
+      delete :destroy, :params => {
+        :object_type => 'issue', :object_id => '2', :user_id => '10'
+      }
+      assert_response :success
+      assert_include 'Watcher removed', response.body
+    end
+    issue.reload
+    assert !issue.watched_by?(group)
+  end
+
   def test_destroy
     @request.session[:user_id] = 2
     assert_difference('Watcher.count', -1) do
diff --git a/test/helpers/avatars_helper_test.rb b/test/helpers/avatars_helper_test.rb
index e02755683..de8573f9b 100644
--- a/test/helpers/avatars_helper_test.rb
+++ b/test/helpers/avatars_helper_test.rb
@@ -43,7 +43,7 @@ class AvatarsHelperTest < Redmine::HelperTest
   end
 
   def test_avatar_with_group
-    assert_nil avatar(Group.first)
+    assert_match %r{src="/images/group.png(\?\d+)?"}, avatar(Group.first)
   end
 
   def test_avatar_with_invalid_arg_should_return_nil
diff --git a/test/unit/group_test.rb b/test/unit/group_test.rb
index 77f79500b..37626ac0e 100644
--- a/test/unit/group_test.rb
+++ b/test/unit/group_test.rb
@@ -23,10 +23,9 @@ class GroupTest < ActiveSupport::TestCase
   fixtures :projects, :trackers, :issue_statuses, :issues,
            :enumerations, :users,
            :projects_trackers,
-           :roles,
-           :member_roles,
-           :members,
-           :groups_users
+           :roles, :member_roles, :members,
+           :groups_users,
+           :watchers
 
   include Redmine::I18n
 
@@ -128,14 +127,21 @@ class GroupTest < ActiveSupport::TestCase
     assert !User.find(8).member_of?(Project.find(5))
   end
 
-  def test_destroy_should_unassign_issues
+  def test_destroy_should_unassign_and_unwatch_issues
     group = Group.find(10)
     Issue.where(:id => 1).update_all(["assigned_to_id = ?", group.id])
+    issue = Issue.find(2)
+    issue.set_watcher(group)
+    issue.save
+    issue.reload
+    assert issue.watcher_user_ids.include?(10)
 
     assert group.destroy
     assert group.destroyed?
 
     assert_nil Issue.find(1).assigned_to_id
+    issue.reload
+    assert !issue.watcher_user_ids.include?(10)
   end
 
   def test_builtin_groups_should_be_created_if_missing
diff --git a/test/unit/query_test.rb b/test/unit/query_test.rb
index 56ec82d14..e99923fab 100644
--- a/test/unit/query_test.rb
+++ b/test/unit/query_test.rb
@@ -421,8 +421,6 @@ class QueryTest < ActiveSupport::TestCase
     issues = find_issues_with_query(query)
     assert issues.any?
     assert_nil issues.detect {|issue| !issue.is_private?}
-  ensure
-    User.current = nil
   end
 
   def test_operator_is_not_on_is_private_field
@@ -436,8 +434,6 @@ class QueryTest < ActiveSupport::TestCase
     issues = find_issues_with_query(query)
     assert issues.any?
     assert_nil issues.detect {|issue| issue.is_private?}
-  ensure
-    User.current = nil
   end
 
   def test_operator_greater_than
@@ -950,7 +946,20 @@ class QueryTest < ActiveSupport::TestCase
     assert_not_nil result
     assert !result.empty?
     assert_equal Issue.visible.watched_by(User.current).sort_by(&:id), result.sort_by(&:id)
-    User.current = nil
+  end
+
+  def test_filter_watched_issues_with_groups_also
+    user = User.find(2)
+    group = Group.find(10)
+    group.users << user
+    Issue.find(3).add_watcher(user)
+    Issue.find(7).add_watcher(group)
+    User.current = user
+    query = IssueQuery.new(:name => '_', :filters => { 'watcher_id' => {:operator => '=', :values => ['me']}})
+    result = find_issues_with_query(query)
+    assert_not_nil result
+    assert !result.empty?
+    assert_equal [3, 7], result.sort_by(&:id).pluck(:id)
   end
 
   def test_filter_unwatched_issues
@@ -960,7 +969,6 @@ class QueryTest < ActiveSupport::TestCase
     assert_not_nil result
     assert !result.empty?
     assert_equal((Issue.visible - Issue.watched_by(User.current)).sort_by(&:id).size, result.sort_by(&:id).size)
-    User.current = nil
   end
 
   def test_filter_on_watched_issues_with_view_issue_watchers_permission
@@ -974,9 +982,6 @@ class QueryTest < ActiveSupport::TestCase
     result = find_issues_with_query(query)
     assert_includes result, Issue.find(1)
     assert_includes result, Issue.find(3)
-  ensure
-    User.current.reload
-    User.current = nil
   end
 
   def test_filter_on_watched_issues_without_view_issue_watchers_permission
@@ -990,9 +995,6 @@ class QueryTest < ActiveSupport::TestCase
     result = find_issues_with_query(query)
     assert_includes result, Issue.find(1)
     assert_not_includes result, Issue.find(3)
-  ensure
-    User.current.reload
-    User.current = nil
   end
 
   def test_filter_on_custom_field_should_ignore_projects_with_field_disabled
diff --git a/test/unit/watcher_test.rb b/test/unit/watcher_test.rb
index 761bf5a71..48e619807 100644
--- a/test/unit/watcher_test.rb
+++ b/test/unit/watcher_test.rb
@@ -40,9 +40,12 @@ class WatcherTest < ActiveSupport::TestCase
   end
 
   def test_watch
+    group = Group.find(10)
+    assert @issue.add_watcher(group)
     assert @issue.add_watcher(@user)
     @issue.reload
     assert @issue.watchers.detect {|w| w.user == @user}
+    assert @issue.watchers.detect {|w| w.user == group}
   end
 
   def test_cant_watch_twice
@@ -103,12 +106,14 @@ class WatcherTest < ActiveSupport::TestCase
   def test_addable_watcher_users
     addable_watcher_users = @issue.addable_watcher_users
     assert_kind_of Array, addable_watcher_users
-    assert_kind_of User, addable_watcher_users.first
+    addable_watcher_users.each do |addable_watcher|
+      assert_equal true, addable_watcher.is_a?(User) || addable_watcher.is_a?(Group)
+    end
   end
 
   def test_addable_watcher_users_should_not_include_user_that_cannot_view_the_object
     issue = Issue.new(:project => Project.find(1), :is_private => true)
-    assert_nil issue.addable_watcher_users.detect {|user| !issue.visible?(user)}
+    assert_nil issue.addable_watcher_users.detect {|user| user.is_a?(User) && !issue.visible?(user)}
   end
 
   def test_any_watched_should_return_false_if_no_object_is_watched
@@ -147,9 +152,12 @@ class WatcherTest < ActiveSupport::TestCase
   end
 
   def test_unwatch
+    group = Group.find(10)
+    assert @issue.add_watcher(group)
     assert @issue.add_watcher(@user)
     @issue.reload
     assert_equal 1, @issue.remove_watcher(@user)
+    assert_equal 1, @issue.remove_watcher(group)
   end
 
   def test_prune_with_user