From b47a90b42413e001fbaeb84c1b4d76a3ec86c607 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20Sch=C3=A4fer?= Date: Mon, 3 Oct 2011 09:52:00 +0200 Subject: [PATCH] Allow to filter watchers by more than just "me". #566 --- app/models/query.rb | 22 ++++++++-- test/unit/query_test.rb | 95 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 114 insertions(+), 3 deletions(-) diff --git a/app/models/query.rb b/app/models/query.rb index 052990ba..8056ffbd 100644 --- a/app/models/query.rb +++ b/app/models/query.rb @@ -208,7 +208,10 @@ class Query < ActiveRecord::Base @available_filters["assigned_to_role"] = { :type => :list_optional, :order => 7, :values => role_values } unless role_values.empty? if User.current.logged? - @available_filters["watcher_id"] = { :type => :list, :order => 15, :values => [["<< #{l(:label_me)} >>", "me"]] } + # populate the watcher list with the same user list as other user filters if the user has the :view_issue_watchers permission in at least one project + # TODO: this could be differentiated more, e.g. all users could watch issues in public projects, but won't necessarily be shown here + watcher_values = User.current.allowed_to_globally?(:view_issue_watchers, {}) ? user_values : [["<< #{l(:label_me)} >>", "me"]] + @available_filters["watcher_id"] = { :type => :list, :order => 15, :values => watcher_values } end if project @@ -442,8 +445,21 @@ class Query < ActiveRecord::Base elsif field == 'watcher_id' db_table = Watcher.table_name db_field = 'user_id' - sql << "#{Issue.table_name}.id #{ operator == '=' ? 'IN' : 'NOT IN' } (SELECT #{db_table}.watchable_id FROM #{db_table} WHERE #{db_table}.watchable_type='Issue' AND " - sql << sql_for_field(field, '=', v, db_table, db_field) + ')' + if User.current.admin? + # Admins can always see all watchers + sql << "#{Issue.table_name}.id #{operator == '=' ? 'IN' : 'NOT IN'} (SELECT #{db_table}.watchable_id FROM #{db_table} WHERE #{db_table}.watchable_type='Issue' AND #{sql_for_field field, '=', v, db_table, db_field})" + else + sql_parts = [] + if User.current.logged? && user_id = v.delete(User.current.id.to_s) + # a user can always see his own watched issues + sql_parts << "#{Issue.table_name}.id #{operator == '=' ? 'IN' : 'NOT IN'} (SELECT #{db_table}.watchable_id FROM #{db_table} WHERE #{db_table}.watchable_type='Issue' AND #{sql_for_field field, '=', [user_id], db_table, db_field})" + end + # filter watchers only in projects the user has the permission to view watchers in + project_ids = User.current.projects_by_role.collect {|r,p| p if r.permissions.include? :view_issue_watchers}.flatten.compact.collect(&:id).uniq + sql_parts << "#{Issue.table_name}.id #{operator == '=' ? 'IN' : 'NOT IN'} (SELECT #{db_table}.watchable_id FROM #{db_table} WHERE #{db_table}.watchable_type='Issue' AND #{sql_for_field field, '=', v, db_table, db_field})"\ + " AND #{Project.table_name}.id IN (#{project_ids.join(',')})" unless project_ids.empty? + sql << "(#{sql_parts.join(' OR ')})" + end elsif field == "member_of_group" # named field if operator == '*' # Any group groups = Group.all diff --git a/test/unit/query_test.rb b/test/unit/query_test.rb index 0acd4754..e49fc0d0 100644 --- a/test/unit/query_test.rb +++ b/test/unit/query_test.rb @@ -444,6 +444,101 @@ class QueryTest < ActiveSupport::TestCase end + context "'watcher_id' filter" do + context "globally" do + context "for an anonymous user" do + should "not be present" do + assert ! @query.available_filters.keys.include?("watcher_id") + end + end + + context "for a logged in user" do + setup do + User.current = User.find 1 + end + + teardown do + User.current = nil + end + + should "be present" do + assert @query.available_filters.keys.include?("watcher_id") + end + + should "be a list" do + assert_equal :list, @query.available_filters["watcher_id"][:type] + end + + should "have a list of active users as values" do + assert @query.available_filters["watcher_id"][:values].include?(["<< me >>", "me"]) + assert @query.available_filters["watcher_id"][:values].include?(["John Smith", "2"]) + assert @query.available_filters["watcher_id"][:values].include?(["Dave Lopper", "3"]) + assert @query.available_filters["watcher_id"][:values].include?(["redMine Admin", "1"]) + assert @query.available_filters["watcher_id"][:values].include?(["User Misc", "8"]) + end + + should "not include active users not member of any project" do + assert ! @query.available_filters["watcher_id"][:values].include?(['Robert Hill','4']) + end + + should "not include locked users as values" do + assert ! @query.available_filters["watcher_id"][:values].include?(['Dave2 Lopper2','5']) + end + + should "not include the anonymous user as values" do + assert ! @query.available_filters["watcher_id"][:values].include?(['Anonymous','6']) + end + end + end + + context "in a project" do + setup do + @query.project = Project.find(1) + end + + context "for an anonymous user" do + should "not be present" do + assert ! @query.available_filters.keys.include?("watcher_id") + end + end + + context "for a logged in user" do + setup do + User.current = User.find 1 + end + + teardown do + User.current = nil + end + + should "be present" do + assert @query.available_filters.keys.include?("watcher_id") + end + + should "be a list" do + assert_equal :list, @query.available_filters["watcher_id"][:type] + end + + should "have a list of the project members as values" do + assert @query.available_filters["watcher_id"][:values].include?(["<< me >>", "me"]) + assert @query.available_filters["watcher_id"][:values].include?(["John Smith", "2"]) + assert @query.available_filters["watcher_id"][:values].include?(["Dave Lopper", "3"]) + end + + should "not include non-project members as values" do + assert ! @query.available_filters["watcher_id"][:values].include?(["redMine Admin", "1"]) + end + + should "not include locked project members as values" do + assert ! @query.available_filters["watcher_id"][:values].include?(['Dave2 Lopper2','5']) + end + + should "not include the anonymous user as values" do + assert ! @query.available_filters["watcher_id"][:values].include?(['Anonymous','6']) + end + end + end + end end context "#statement" do