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

15 files changed, 161 insertions, 62 deletions

diff --git a/app/controllers/watchers_controller.rb b/app/controllers/watchers_controller.rb
index f0692e31d..e0de27464 100644
--- a/app/controllers/watchers_controller.rb
+++ b/app/controllers/watchers_controller.rb
@@ -42,7 +42,9 @@ class WatchersController < ApplicationController
     else
       user_ids << params[:user_id]
     end
-    users = User.active.visible.where(:id => user_ids.flatten.compact.uniq)
+    user_ids = user_ids.flatten.compact.uniq
+    users = User.active.visible.where(:id => user_ids).to_a
+    users += Group.givable.active.visible.where(:id => user_ids).to_a
     users.each do |user|
       @watchables.each do |watchable|
         Watcher.create(:watchable => watchable, :user => user)
@@ -59,6 +61,7 @@ class WatchersController < ApplicationController
     if params[:watcher]
       user_ids = params[:watcher][:user_ids] || [params[:watcher][:user_id]]
       @users = User.active.visible.where(:id => user_ids).to_a
+      @users += Group.givable.active.visible.where(:id => user_ids).to_a
     end
     if @users.blank?
       head 200
@@ -66,7 +69,7 @@ class WatchersController < ApplicationController
   end
 
   def destroy
-    user = User.find(params[:user_id])
+    user = Principal.find(params[:user_id])
     @watchables.each do |watchable|
       watchable.set_watcher(user, false)
     end
@@ -119,13 +122,16 @@ class WatchersController < ApplicationController
   end
 
   def users_for_new_watcher
-    scope = nil
+    scope, scope_groups = nil
     if params[:q].blank? && @project.present?
       scope = @project.users
+      scope_groups = @project.principals.merge(Group.givable)
     else
       scope = User.all.limit(100)
+      scope_groups = Group.givable.limit(100)
     end
     users = scope.active.visible.sorted.like(params[:q]).to_a
+    users += scope_groups.active.visible.sorted.like(params[:q]).to_a
     if @watchables && @watchables.size == 1
       users -= @watchables.first.watcher_users
     end
diff --git a/app/helpers/avatars_helper.rb b/app/helpers/avatars_helper.rb
index 7e2a1fffd..49be006a8 100644
--- a/app/helpers/avatars_helper.rb
+++ b/app/helpers/avatars_helper.rb
@@ -51,6 +51,8 @@ module AvatarsHelper
         gravatar(email.to_s.downcase, options) rescue nil
       elsif user.is_a?(AnonymousUser)
         anonymous_avatar(options)
+      elsif user.is_a?(Group)
+        group_avatar(options)
       else
         nil
       end
@@ -72,4 +74,8 @@ module AvatarsHelper
   def anonymous_avatar(options={})
     image_tag 'anonymous.png', GravatarHelper::DEFAULT_OPTIONS.except(:default, :rating, :ssl).merge(options)
   end
+
+  def group_avatar(options={})
+    image_tag 'group.png', GravatarHelper::DEFAULT_OPTIONS.except(:default, :rating, :ssl).merge(options)
+  end
 end
diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb
index 2ea81d566..0f327b77e 100644
--- a/app/helpers/issues_helper.rb
+++ b/app/helpers/issues_helper.rb
@@ -365,8 +365,11 @@ module IssuesHelper
   # on the new issue form
   def users_for_new_issue_watchers(issue)
     users = issue.watcher_users.select{|u| u.status == User::STATUS_ACTIVE}
-    if issue.project.users.count <= 20
-      users = (users + issue.project.users.sort).uniq
+    project = issue.project
+    scope_users = project.users
+    scope_groups = project.principals.merge(Group.givable)
+    if scope_users.count + scope_groups.count <= 20
+      users = (users + scope_users.sort + scope_groups.sort).uniq
     end
     users
   end
diff --git a/app/helpers/watchers_helper.rb b/app/helpers/watchers_helper.rb
index dab5e6b76..ac732c339 100644
--- a/app/helpers/watchers_helper.rb
+++ b/app/helpers/watchers_helper.rb
@@ -47,7 +47,7 @@ module WatchersHelper
   def watchers_list(object)
     remove_allowed = User.current.allowed_to?("delete_#{object.class.name.underscore}_watchers".to_sym, object.project)
     content = ''.html_safe
-    lis = object.watcher_users.preload(:email_address).collect do |user|
+    lis = object.watcher_users.collect do |user|
       s = ''.html_safe
       s << avatar(user, :size => "16").to_s
       s << link_to_user(user, :class => 'user')
diff --git a/app/models/group.rb b/app/models/group.rb
index 21166f1af..c1106e07f 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -115,6 +115,7 @@ class Group < Principal
     return if self.id.nil?
 
     Issue.where(['assigned_to_id = ?', id]).update_all('assigned_to_id = NULL')
+    Watcher.where('user_id = ?', id).delete_all
   end
 end
 
diff --git a/app/models/query.rb b/app/models/query.rb
index 0e38b47d8..5672a6e9e 100644
--- a/app/models/query.rb
+++ b/app/models/query.rb
@@ -600,7 +600,7 @@ class Query < ActiveRecord::Base
 
   def watcher_values
     watcher_values = [["<< #{l(:label_me)} >>", "me"]]
-    watcher_values += users.sort_by(&:status).collect{|s| [s.name, s.id.to_s, l("status_#{User::LABEL_BY_STATUS[s.status]}")] } if User.current.allowed_to?(:view_issue_watchers, self.project)
+    watcher_values += principals.sort_by(&:status).collect{|s| [s.name, s.id.to_s, l("status_#{User::LABEL_BY_STATUS[s.status]}")] } if User.current.allowed_to?(:view_issue_watchers, self.project)
     watcher_values
   end
 
@@ -913,7 +913,7 @@ class Query < ActiveRecord::Base
         if v.delete("me")
           if User.current.logged?
             v.push(User.current.id.to_s)
-            v += User.current.group_ids.map(&:to_s) if field == 'assigned_to_id'
+            v += User.current.group_ids.map(&:to_s) if %w(assigned_to_id watcher_id).include?(field)
           else
             v.push("0")
           end
diff --git a/app/models/watcher.rb b/app/models/watcher.rb
index 8ea457060..15bfaa6db 100644
--- a/app/models/watcher.rb
+++ b/app/models/watcher.rb
@@ -19,7 +19,7 @@
 
 class Watcher < ActiveRecord::Base
   belongs_to :watchable, :polymorphic => true
-  belongs_to :user
+  belongs_to :user, :class_name => 'Principal'
 
   validates_presence_of :user
   validates_uniqueness_of :user_id, :scope => [:watchable_type, :watchable_id]
@@ -54,7 +54,8 @@ class Watcher < ActiveRecord::Base
   protected
 
   def validate_user
-    errors.add :user_id, :invalid unless user.nil? || user.active?
+    errors.add :user_id, :invalid \
+      unless user.nil? || (user.is_a?(User) && user.active?) || (user.is_a?(Group) && user.givable?)
   end
 
   def self.prune_single_user(user, options={})
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 1ebe9e2fc..e37151d8f 100644
--- a/lib/plugins/acts_as_watchable/lib/acts_as_watchable.rb
+++ b/lib/plugins/acts_as_watchable/lib/acts_as_watchable.rb
@@ -31,9 +31,9 @@ module Redmine
 
         # Returns an array of users that are proposed as watchers
         def addable_watcher_users
-          users = self.project.users.sort - self.watcher_users
+          users = (self.project.users.sort + self.project.principals.merge(Group.givable).sort) - self.watcher_users
           if respond_to?(:visible?)
-            users.reject! {|user| !visible?(user)}
+            users.reject! {|user| user.is_a?(User) && !visible?(user)}
           end
           users
         end
@@ -47,7 +47,7 @@ module Redmine
 
         # Removes user from the watchers list
         def remove_watcher(user)
-          return nil unless user && user.is_a?(User)
+          return nil unless user && (user.is_a?(User) || user.is_a?(Group))
           # Rails does not reset the has_many :through association
           watcher_users.reset
           watchers.where(:user_id => user.id).delete_all
@@ -73,6 +73,8 @@ module Redmine
 
         def notified_watchers
           notified = watcher_users.active.to_a
+          notified = notified.map {|n| n.is_a?(Group) ? n.users : n}.flatten
+          notified.uniq!
           notified.reject! {|user| user.mail.blank? || user.mail_notification == 'none'}
           if respond_to?(:visible?)
             notified.reject! {|user| !visible?(user)}
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