Add ProjectsFinder where applicabledefault-scope-to-explicit

11 files changed, 42 insertions, 29 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 7ec9580ff94..fb932c68722 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,6 +1,7 @@
 Please view this file on the master branch, on stable branches it's out of date.
 
 v 8.8.0 (unreleased)
+  - Excluding projects pending deletion is now down using explicit scoping
   - Snippets tab under user profile. !4001 (Long Nguyen)
   - Fix error when using link to uploads in global snippets
   - Assign labels and milestone to target project when moving issue. !3934 (Long Nguyen)
@@ -46,7 +47,6 @@ v 8.8.0 (unreleased)
 
 v 8.7.5
   - Fix relative links in wiki pages. !4050
-  - Excluding projects pending deletion is now down using explicit scoping
 
 v 8.7.4
   - Links for Redmine issue references are generated correctly again !4048 (Benedikt Huss)
diff --git a/app/controllers/admin/dashboard_controller.rb b/app/controllers/admin/dashboard_controller.rb
index c44ed2c6c08..a302de55fcb 100644
--- a/app/controllers/admin/dashboard_controller.rb
+++ b/app/controllers/admin/dashboard_controller.rb
@@ -1,6 +1,6 @@
 class Admin::DashboardController < Admin::ApplicationController
   def index
-    @projects = Project.without_pending_delete.limit(10)
+    @projects = ProjectsFinder.execute(current_user, scope: all).limit(10)
     @users = User.limit(10)
     @groups = Group.limit(10)
   end
diff --git a/app/controllers/admin/projects_controller.rb b/app/controllers/admin/projects_controller.rb
index d1e620142ba..4cf4ffc2791 100644
--- a/app/controllers/admin/projects_controller.rb
+++ b/app/controllers/admin/projects_controller.rb
@@ -3,7 +3,7 @@ class Admin::ProjectsController < Admin::ApplicationController
   before_action :group, only: [:show, :transfer]
 
   def index
-    @projects = Project.without_pending_delete
+    @projects = ProjectsFinder.execute(current_user, scope: :all)
     @projects = @projects.in_namespace(params[:namespace_id]) if params[:namespace_id].present?
     @projects = @projects.where("projects.visibility_level IN (?)", params[:visibility_levels]) if params[:visibility_levels].present?
     @projects = @projects.with_push if params[:with_push].present?
diff --git a/app/controllers/admin/runners_controller.rb b/app/controllers/admin/runners_controller.rb
index 2eec555c36e..67be2b28636 100644
--- a/app/controllers/admin/runners_controller.rb
+++ b/app/controllers/admin/runners_controller.rb
@@ -12,9 +12,9 @@ class Admin::RunnersController < Admin::ApplicationController
     @builds = @runner.builds.order('id DESC').first(30)
     @projects =
       if params[:search].present?
-        ::Project.without_pending_delete.search(params[:search])
+        ProjectsFinder.execute(current_user, scope: :all).search(params[:search])
       else
-        Project.without_pending_delete
+        ProjectsFinder.execute(current_user, scope: :all)
       end
     @projects = @projects.where.not(id: @runner.projects.select(:id)) if @runner.projects.any?
     @projects = @projects.page(params[:page]).per(30)
diff --git a/app/controllers/dashboard/application_controller.rb b/app/controllers/dashboard/application_controller.rb
index bf82fa06995..5c9e0610781 100644
--- a/app/controllers/dashboard/application_controller.rb
+++ b/app/controllers/dashboard/application_controller.rb
@@ -4,6 +4,7 @@ class Dashboard::ApplicationController < ApplicationController
   private
 
   def projects
-    @projects ||= ProjectsFinder.new.execute(current_user).sorted_by_activity.non_archived
+    @projects ||= ProjectsFinder.execute(current_user, scope: :authorized).
+                                  sorted_by_activity.non_archived
   end
 end
diff --git a/app/controllers/dashboard/projects_controller.rb b/app/controllers/dashboard/projects_controller.rb
index f606793defb..6e98b330bf5 100644
--- a/app/controllers/dashboard/projects_controller.rb
+++ b/app/controllers/dashboard/projects_controller.rb
@@ -4,7 +4,7 @@ class Dashboard::ProjectsController < Dashboard::ApplicationController
   before_action :event_filter
 
   def index
-    @projects = current_user.authorized_projects.without_pending_delete.sorted_by_activity
+    @projects = ProjectsFinder.execute(current_user, scope: :authorized).sorted_by_activity
     @projects = filter_projects(@projects)
     @projects = @projects.includes(:namespace)
     @projects = @projects.sort(@sort = params[:sort])
@@ -28,7 +28,7 @@ class Dashboard::ProjectsController < Dashboard::ApplicationController
   end
 
   def starred
-    @projects = current_user.viewable_starred_projects.without_pending_delete.sorted_by_activity
+    @projects = ProjectsFinder.execute(current_user, scope: :viewable_starred_projects).sorted_by_activity
     @projects = filter_projects(@projects)
     @projects = @projects.includes(:namespace, :forked_from_project, :tags)
     @projects = @projects.sort(@sort = params[:sort])
diff --git a/app/finders/projects_finder.rb b/app/finders/projects_finder.rb
index be30adce665..55133d0ad00 100644
--- a/app/finders/projects_finder.rb
+++ b/app/finders/projects_finder.rb
@@ -1,18 +1,21 @@
 class ProjectsFinder < UnionFinder
-  def execute(current_user = nil, options = {})
-    segments = all_projects(current_user)
+  # Used for:
+  #   - all projects (Admin)            => :all
+  #   - projects I can access           => :authorized
+  #   - visible to me or public         => :public_to_user
+  def self.execute(current_user = nil, scope: :public_to_user)
+    case scope
+    when :all
+      Project.without_pending_delete
+    when :authorized
+      return self.execute unless current_user
 
-    find_union(segments, Project)
-  end
-
-  private
-
-  def all_projects(current_user)
-    projects = []
-
-    projects << current_user.authorized_projects if current_user
-    projects << Project.without_pending_delete.public_to_user(current_user)
-
-    projects
+      current_user.authorized_projects.without_pending_delete
+    when :viewable_starred_projects
+      current_user.viewable_starred_projects.without_pending_delete
+    when :public_to_user
+      # #public_to_user(nil) will yield public projects only
+      Project.without_pending_delete.public_to_user(current_user)
+    end
   end
 end
diff --git a/app/views/admin/dashboard/index.html.haml b/app/views/admin/dashboard/index.html.haml
index 19ae12848b8..fa84a52f954 100644
--- a/app/views/admin/dashboard/index.html.haml
+++ b/app/views/admin/dashboard/index.html.haml
@@ -104,7 +104,7 @@
         %h4 Projects
         .data
           = link_to admin_namespaces_projects_path do
-            %h1= number_with_delimiter(Project.without_pending_delete.count)
+            %h1= number_with_delimiter(ProjectsFinder.execute(current_user, scope: all).count)
           %hr
           = link_to('New Project', new_project_path, class: "btn btn-new")
     .col-sm-4
diff --git a/lib/api/projects.rb b/lib/api/projects.rb
index 5e9e14ad00a..13f81e9506f 100644
--- a/lib/api/projects.rb
+++ b/lib/api/projects.rb
@@ -22,10 +22,10 @@ module API
       # Example Request:
       #   GET /projects
       get do
-        @projects = current_user.authorized_projects
-        @projects = filter_projects(@projects)
-        @projects = paginate @projects
-        present @projects, with: Entities::ProjectWithAccess, user: current_user
+        projects = ProjectsFinder.execute(current_user, scope: :authorized)
+        projects = filter_projects(projects)
+        projects = paginate(projects)
+        present projects, with: Entities::ProjectWithAccess, user: current_user
       end
 
       # Get an owned projects list for authenticated user
@@ -43,6 +43,7 @@ module API
       #
       # Example Request:
       #   GET /projects/starred
+      # TODO ZJ -- Use the ProjectsFinder here
       get '/starred' do
         @projects = current_user.viewable_starred_projects
         @projects = filter_projects(@projects)
@@ -56,7 +57,7 @@ module API
       #   GET /projects/all
       get '/all' do
         authenticated_as_admin!
-        @projects = Project.without_pending_delete
+        @projects = ProjectsFinder.execute(current_user, scope: :all)
         @projects = filter_projects(@projects)
         @projects = paginate @projects
         present @projects, with: Entities::ProjectWithAccess, user: current_user
diff --git a/lib/gitlab/search_results.rb b/lib/gitlab/search_results.rb
index 31a7536b431..78ac2e91696 100644
--- a/lib/gitlab/search_results.rb
+++ b/lib/gitlab/search_results.rb
@@ -8,7 +8,7 @@ module Gitlab
 
     def initialize(current_user, limit_projects, query)
       @current_user = current_user
-      @limit_projects = limit_projects || Project.without_pending_delete
+      @limit_projects = limit_projects || ProjectFinder.execute(current_user, scope: :authorized)
       @query = Shellwords.shellescape(query) if query.present?
     end
 
diff --git a/spec/finders/projects_finder_spec.rb b/spec/finders/projects_finder_spec.rb
index 0a1cc3b3df7..d199e8787ff 100644
--- a/spec/finders/projects_finder_spec.rb
+++ b/spec/finders/projects_finder_spec.rb
@@ -91,5 +91,13 @@ describe ProjectsFinder do
         end
       end
     end
+
+    describe 'as admin' do
+      it 'returns all projects' do
+        admin = create(:user, :admin)
+
+        expect(ProjectsFinder.new.execute(admin)).to eq([shared_project, public_project, internal_project, private_project])
+      end
+    end
   end
 end