Some refactoring of User.with_permission

app/helpers/additionals_issues_helper.rb

@@ -1,7 +1,7 @@
 module AdditionalsIssuesHelper
   def author_options_for_select(project, entity = nil, permission = nil)
     scope = project.present? ? project.users.visible : User.active.visible
-    scope = User.with_permission(scope, permission, project) unless permission.nil?
+    scope = scope.with_permission(permission, project) unless permission.nil?
     authors = scope.sorted.to_a
 
     unless entity.nil?

lib/additionals/patches/user_patch.rb

@@ -8,26 +8,31 @@ module Additionals
       end
 
       class_methods do
-        def with_permission(users, permission, project)
+        # NOTE: this is a better (performance related) solution as:
+        # authors = users.to_a.select { |u| u.allowed_to? permission, project, global: project.nil? }
+        def with_permission(permission, project = nil)
           # Clear cache for debuging performance issue
           # ActiveRecord::Base.connection.clear_query_cache
 
-          # TODO: find a better solution with better performance
-          # authors = users.to_a.select { |u| u.allowed_to? permission, project, global: project.nil? }
-
           role_ids = Role.builtin(false).select { |p| p.permissions.include? permission }
           role_ids.map!(&:id)
 
           admin_ids = User.visible.active.where(admin: true).ids
 
-          member_scope = Member.joins(:member_roles).active.where(user_id: users.ids).where(member_roles: { role_id: role_ids })
+          member_scope = Member.joins(:member_roles)
+                               .joins(:project)
+                               .active
+                               .where(projects: { status: Project::STATUS_ACTIVE })
+                               .where(user_id: all.ids)
+                               .where(member_roles: { role_id: role_ids })
+                               .distinct
 
           if project.nil?
-            ids = member_scope.map(&:user_id) | admin_ids
-            users.where(id: ids)
+            ids = member_scope.pluck(:user_id) | admin_ids
+            where(id: ids)
           else
-            member_ids = member_scope.where(project_id: project).map(&:user_id)
-            users.where(id: member_ids).or(users.where(id: admin_ids))
+            member_ids = member_scope.where(project_id: project).pluck(:user_id)
+            where(id: member_ids).or(where(id: admin_ids))
           end
         end
       end

test/unit/role_test.rb

@@ -10,7 +10,7 @@ class RoleTest < Additionals::TestCase
     prepare_tests
   end
 
-  def test_create
+  def test_with_permission
     role = Role.new(name: 'role without hide')
     assert role.save
 

test/unit/user_test.rb

@@ -0,0 +1,30 @@
+require File.expand_path('../../test_helper', __FILE__)
+
+class UserTest < Additionals::TestCase
+  fixtures :users, :email_addresses, :members, :projects, :roles, :member_roles, :auth_sources,
+           :trackers, :issue_statuses,
+           :projects_trackers,
+           :watchers,
+           :issue_categories, :enumerations, :issues,
+           :journals, :journal_details,
+           :groups_users,
+           :enabled_modules,
+           :tokens,
+           :user_preferences
+
+  def setup
+    prepare_tests
+  end
+
+  def test_with_permission
+    admin_user = User.generate!(admin: true)
+
+    users = User.visible.active.with_permission(:save_dashboards)
+    assert_equal 5, users.count
+    assert users.exists?(id: admin_user)
+  end
+
+  def test_with_permission_on_project
+    assert_equal 3, User.visible.active.with_permission(:save_dashboards, projects(:projects_001)).count
+  end
+end