Merge remote-tracking branch 'refs/remotes/thegcat/feature/566-more_watchers_to_filter_by'

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

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