Nemcio/Redmine
1
0
Fork 0
mirror of https://github.com/redmine/redmine.git synced 2025-11-02 03:15:57 +01:00
Code Issues Packages Projects Releases Activity

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

Browse Source 
Patch by Yuichi HARADA.


git-svn-id: http://svn.redmine.org/redmine/trunk@19498 e93f8b46-1217-0410-a6f0-8f06a7374b81
This commit is contained in:
Go MAEDA
2020-02-06 08:19:13 +00:00
parent 29433771b4
commit b995014c49
15 changed files with 161 additions and 62 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
app/controllers/watchers_controller.rb
View File

@@ -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

6
app/helpers/avatars_helper.rb
View File

@@ -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

7
app/helpers/issues_helper.rb
View File

@@ -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

2
app/helpers/watchers_helper.rb
View File

@@ -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')

1
app/models/group.rb
View File

@@ -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

4
app/models/query.rb
View File

@@ -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

5
app/models/watcher.rb
View File

@@ -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={})

8
lib/plugins/acts_as_watchable/lib/acts_as_watchable.rb
View File

@@ -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)}

54
test/functional/issues_controller_test.rb
View File

@@ -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

5
test/functional/queries_controller_test.rb
View File

@@ -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

63
test/functional/watchers_controller_test.rb
View File

@@ -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

2
test/helpers/avatars_helper_test.rb
View File

@@ -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

16
test/unit/group_test.rb
View File

@@ -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

26
test/unit/query_test.rb
View File

@@ -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

12
test/unit/watcher_test.rb
View File

@@ -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