Only do complicated confidentiality checks when necessary

When we are filtering by a single project, and the current user has access to
see confidential issues on that project, we don't need to filter by
confidentiality at all - just as if the user were an admin.

The filter by confidentiality often picks a non-optimal query plan: for
instance, AND-ing the results of all issues in the project (a relatively small
set), and all issues in the states requested (a huge set), rather than just
starting small and winnowing further.

1 files changed, 28 insertions, 14 deletions

diff --git a/app/finders/issues_finder.rb b/app/finders/issues_finder.rb
index 3da5508aefd..3455a75b8bc 100644
--- a/app/finders/issues_finder.rb
+++ b/app/finders/issues_finder.rb
@@ -16,14 +16,30 @@
 #     sort: string
 #
 class IssuesFinder < IssuableFinder
+  CONFIDENTIAL_ACCESS_LEVEL = Gitlab::Access::REPORTER
+
   def klass
     Issue
   end
 
+  def not_restricted_by_confidentiality
+    return Issue.where('issues.confidential IS NOT TRUE') if user_cannot_see_confidential_issues?
+    return Issue.all if user_can_see_all_confidential_issues?
+
+    Issue.where('
+      issues.confidential IS NOT TRUE
+      OR (issues.confidential = TRUE
+        AND (issues.author_id = :user_id
+          OR EXISTS (SELECT TRUE FROM issue_assignees WHERE user_id = :user_id AND issue_id = issues.id)
+          OR issues.project_id IN(:project_ids)))',
+      user_id: current_user.id,
+      project_ids: current_user.authorized_projects(CONFIDENTIAL_ACCESS_LEVEL).select(:id))
+  end
+
   private
 
   def init_collection
-    IssuesFinder.not_restricted_by_confidentiality(current_user)
+    not_restricted_by_confidentiality
   end
 
   def by_assignee(items)
@@ -38,22 +54,20 @@ class IssuesFinder < IssuableFinder
     end
   end
 
-  def self.not_restricted_by_confidentiality(user)
-    return Issue.where('issues.confidential IS NOT TRUE') if user.blank?
+  def item_project_ids(items)
+    items&.reorder(nil)&.select(:project_id)
+  end
 
-    return Issue.all if user.full_private_access?
+  def user_can_see_all_confidential_issues?
+    return false unless current_user
+    return true if current_user.full_private_access?
 
-    Issue.where('
-      issues.confidential IS NOT TRUE
-      OR (issues.confidential = TRUE
-        AND (issues.author_id = :user_id
-          OR EXISTS (SELECT TRUE FROM issue_assignees WHERE user_id = :user_id AND issue_id = issues.id)
-          OR issues.project_id IN(:project_ids)))',
-      user_id: user.id,
-      project_ids: user.authorized_projects(Gitlab::Access::REPORTER).select(:id))
+    project? &&
+      project &&
+      project.team.max_member_access(current_user.id) >= CONFIDENTIAL_ACCESS_LEVEL
   end
 
-  def item_project_ids(items)
-    items&.reorder(nil)&.select(:project_id)
+  def user_cannot_see_confidential_issues?
+    current_user.blank?
   end
 end