From b996a82ff44e3bcad5e5fb70cabbfa808d06cf62 Mon Sep 17 00:00:00 2001
From: Jacopo <beschi.jacopo@gmail.com>
Date: Fri, 3 Mar 2017 11:35:04 +0100
Subject: ProjectsFinder should handle more options

Extended ProjectFinder in order to handle the following options:
 - current_user - which user use
 - project_ids_relation: int[] - project ids to use
 - params:
   -  trending: boolean
   -  non_public: boolean
   -  starred: boolean
   -  sort: string
   -  visibility_level: int
   -  tags: string[]
   -  personal: boolean
   -  search: string
   -  non_archived: boolean

GroupProjectsFinder now inherits from ProjectsFinder.
Changed the code in order to use the new available options.
---
 app/controllers/admin/projects_controller.rb       |   1 +
 app/controllers/concerns/filter_projects.rb        |  17 ---
 .../concerns/params_backward_compatibility.rb      |   7 ++
 app/controllers/dashboard/projects_controller.rb   |  27 +++--
 app/controllers/explore/projects_controller.rb     |  34 +++---
 app/controllers/groups/application_controller.rb   |   2 +-
 app/controllers/groups_controller.rb               |  11 +-
 app/controllers/projects/forks_controller.rb       |   2 +-
 app/controllers/users_controller.rb                |   2 +-
 app/finders/group_projects_finder.rb               |  59 +++++++---
 app/finders/issuable_finder.rb                     |   8 +-
 app/finders/labels_finder.rb                       |   2 +-
 app/finders/projects_finder.rb                     |  88 ++++++++++++--
 app/finders/todos_finder.rb                        |   2 +-
 app/helpers/sorting_helper.rb                      |  20 +++-
 app/models/project.rb                              |   7 +-
 app/policies/group_policy.rb                       |   4 +-
 app/services/search/global_service.rb              |   2 +-
 .../projects/_zero_authorized_projects.html.haml   |   2 +-
 app/views/shared/projects/_dropdown.html.haml      |   2 +-
 ...10-projectfinder-should-handle-more-options.yml |   4 +
 lib/api/groups.rb                                  |   2 +-
 lib/api/projects.rb                                |   2 +-
 lib/api/users.rb                                   |   2 +-
 lib/api/v3/groups.rb                               |   2 +-
 lib/api/v3/projects.rb                             |   2 +-
 lib/api/v3/users.rb                                |   2 +-
 spec/finders/group_projects_finder_spec.rb         |  70 ++++++-----
 spec/finders/projects_finder_spec.rb               | 128 +++++++++++++++++++--
 29 files changed, 370 insertions(+), 143 deletions(-)
 delete mode 100644 app/controllers/concerns/filter_projects.rb
 create mode 100644 app/controllers/concerns/params_backward_compatibility.rb
 create mode 100644 changelogs/unreleased/28810-projectfinder-should-handle-more-options.yml

diff --git a/app/controllers/admin/projects_controller.rb b/app/controllers/admin/projects_controller.rb
index daecfc832bf..a1975c0e341 100644
--- a/app/controllers/admin/projects_controller.rb
+++ b/app/controllers/admin/projects_controller.rb
@@ -3,6 +3,7 @@ class Admin::ProjectsController < Admin::ApplicationController
   before_action :group, only: [:show, :transfer]
 
   def index
+    params[:sort] ||= 'latest_activity_desc'
     @projects = Project.with_statistics
     @projects = @projects.in_namespace(params[:namespace_id]) if params[:namespace_id].present?
     @projects = @projects.where(visibility_level: params[:visibility_level]) if params[:visibility_level].present?
diff --git a/app/controllers/concerns/filter_projects.rb b/app/controllers/concerns/filter_projects.rb
deleted file mode 100644
index 6014112256a..00000000000
--- a/app/controllers/concerns/filter_projects.rb
+++ /dev/null
@@ -1,17 +0,0 @@
-# == FilterProjects
-#
-# Controller concern to handle projects filtering
-# * by name
-# * by archived state
-#
-module FilterProjects
-  extend ActiveSupport::Concern
-
-  def filter_projects(projects)
-    projects = projects.search(params[:name]) if params[:name].present?
-    projects = projects.non_archived if params[:archived].blank?
-    projects = projects.personal(current_user) if params[:personal].present? && current_user
-
-    projects
-  end
-end
diff --git a/app/controllers/concerns/params_backward_compatibility.rb b/app/controllers/concerns/params_backward_compatibility.rb
new file mode 100644
index 00000000000..b0e3d9c7b34
--- /dev/null
+++ b/app/controllers/concerns/params_backward_compatibility.rb
@@ -0,0 +1,7 @@
+module ParamsBackwardCompatibility
+  private
+
+  def set_non_archived_param
+    params[:non_archived] = params[:archived].blank?
+  end
+end
diff --git a/app/controllers/dashboard/projects_controller.rb b/app/controllers/dashboard/projects_controller.rb
index be00d765f73..5a1efcab1a3 100644
--- a/app/controllers/dashboard/projects_controller.rb
+++ b/app/controllers/dashboard/projects_controller.rb
@@ -1,10 +1,11 @@
 class Dashboard::ProjectsController < Dashboard::ApplicationController
-  include FilterProjects
+  include ParamsBackwardCompatibility
+
+  before_action :set_non_archived_param
+  before_action :default_sorting
 
   def index
-    @projects = load_projects(current_user.authorized_projects)
-    @projects = @projects.sort(@sort = params[:sort])
-    @projects = @projects.page(params[:page])
+    @projects = load_projects(params.merge(non_public: true)).page(params[:page])
 
     respond_to do |format|
       format.html { @last_push = current_user.recent_push }
@@ -21,10 +22,8 @@ class Dashboard::ProjectsController < Dashboard::ApplicationController
   end
 
   def starred
-    @projects = load_projects(current_user.viewable_starred_projects)
-    @projects = @projects.includes(:forked_from_project, :tags)
-    @projects = @projects.sort(@sort = params[:sort])
-    @projects = @projects.page(params[:page])
+    @projects = load_projects(params.merge(starred: true)).
+      includes(:forked_from_project, :tags).page(params[:page])
 
     @last_push = current_user.recent_push
     @groups = []
@@ -41,14 +40,18 @@ class Dashboard::ProjectsController < Dashboard::ApplicationController
 
   private
 
-  def load_projects(base_scope)
-    projects = base_scope.sorted_by_activity.includes(:route, namespace: :route)
+  def default_sorting
+    params[:sort] ||= 'latest_activity_desc'
+    @sort = params[:sort]
+  end
 
-    filter_projects(projects)
+  def load_projects(finder_params)
+    ProjectsFinder.new(params: finder_params, current_user: current_user).
+      execute.includes(:route, namespace: :route)
   end
 
   def load_events
-    @events = Event.in_projects(load_projects(current_user.authorized_projects))
+    @events = Event.in_projects(load_projects(params.merge(non_public: true)))
     @events = event_filter.apply_filter(@events).with_associations
     @events = @events.limit(20).offset(params[:offset] || 0)
   end
diff --git a/app/controllers/explore/projects_controller.rb b/app/controllers/explore/projects_controller.rb
index 6167f9bd335..8f1870759e4 100644
--- a/app/controllers/explore/projects_controller.rb
+++ b/app/controllers/explore/projects_controller.rb
@@ -1,14 +1,12 @@
 class Explore::ProjectsController < Explore::ApplicationController
-  include FilterProjects
+  include ParamsBackwardCompatibility
+
+  before_action :set_non_archived_param
 
   def index
-    @projects = load_projects
-    @tags = @projects.tags_on(:tags)
-    @projects = @projects.tagged_with(params[:tag]) if params[:tag].present?
-    @projects = @projects.where(visibility_level: params[:visibility_level]) if params[:visibility_level].present?
-    @projects = filter_projects(@projects)
-    @projects = @projects.sort(@sort = params[:sort])
-    @projects = @projects.includes(:namespace).page(params[:page])
+    params[:sort] ||= 'latest_activity_desc'
+    @sort = params[:sort]
+    @projects = load_projects.page(params[:page])
 
     respond_to do |format|
       format.html
@@ -21,10 +19,9 @@ class Explore::ProjectsController < Explore::ApplicationController
   end
 
   def trending
-    @projects = load_projects(Project.trending)
-    @projects = filter_projects(@projects)
-    @projects = @projects.sort(@sort = params[:sort])
-    @projects = @projects.page(params[:page])
+    params[:trending] = true
+    @sort = params[:sort]
+    @projects = load_projects.page(params[:page])
 
     respond_to do |format|
       format.html
@@ -37,10 +34,7 @@ class Explore::ProjectsController < Explore::ApplicationController
   end
 
   def starred
-    @projects = load_projects
-    @projects = filter_projects(@projects)
-    @projects = @projects.reorder('star_count DESC')
-    @projects = @projects.page(params[:page])
+    @projects = load_projects.reorder('star_count DESC').page(params[:page])
 
     respond_to do |format|
       format.html
@@ -52,10 +46,10 @@ class Explore::ProjectsController < Explore::ApplicationController
     end
   end
 
-  protected
+  private
 
-  def load_projects(base_scope = nil)
-    base_scope ||= ProjectsFinder.new.execute(current_user)
-    base_scope.includes(:route, namespace: :route)
+  def load_projects
+    ProjectsFinder.new(current_user: current_user, params: params).
+      execute.includes(:route, namespace: :route)
   end
 end
diff --git a/app/controllers/groups/application_controller.rb b/app/controllers/groups/application_controller.rb
index 8b69c18d689..29ffaeb19c1 100644
--- a/app/controllers/groups/application_controller.rb
+++ b/app/controllers/groups/application_controller.rb
@@ -27,7 +27,7 @@ class Groups::ApplicationController < ApplicationController
   end
 
   def group_projects
-    @projects ||= GroupProjectsFinder.new(group).execute(current_user)
+    @projects ||= GroupProjectsFinder.new(group: group, current_user: current_user).execute
   end
 
   def authorize_admin_group!
diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb
index 05f9ee1ee90..78c9f1f7004 100644
--- a/app/controllers/groups_controller.rb
+++ b/app/controllers/groups_controller.rb
@@ -1,7 +1,7 @@
 class GroupsController < Groups::ApplicationController
-  include FilterProjects
   include IssuesAction
   include MergeRequestsAction
+  include ParamsBackwardCompatibility
 
   respond_to :html
 
@@ -105,15 +105,16 @@ class GroupsController < Groups::ApplicationController
   protected
 
   def setup_projects
+    set_non_archived_param
+    params[:sort] ||= 'latest_activity_desc'
+    @sort = params[:sort]
+
     options = {}
     options[:only_owned] = true if params[:shared] == '0'
     options[:only_shared] = true if params[:shared] == '1'
 
-    @projects = GroupProjectsFinder.new(group, options).execute(current_user)
+    @projects = GroupProjectsFinder.new(params: params, group: group, options: options, current_user: current_user).execute
     @projects = @projects.includes(:namespace)
-    @projects = @projects.sorted_by_activity
-    @projects = filter_projects(@projects)
-    @projects = @projects.sort(@sort = params[:sort])
     @projects = @projects.page(params[:page]) if params[:name].blank?
   end
 
diff --git a/app/controllers/projects/forks_controller.rb b/app/controllers/projects/forks_controller.rb
index ba46e2528e6..1eb3800e49d 100644
--- a/app/controllers/projects/forks_controller.rb
+++ b/app/controllers/projects/forks_controller.rb
@@ -9,7 +9,7 @@ class Projects::ForksController < Projects::ApplicationController
   def index
     base_query = project.forks.includes(:creator)
 
-    @forks               = base_query.merge(ProjectsFinder.new.execute(current_user))
+    @forks               = base_query.merge(ProjectsFinder.new(current_user: current_user).execute)
     @total_forks_count   = base_query.size
     @private_forks_count = @total_forks_count - @forks.size
     @public_forks_count  = @total_forks_count - @private_forks_count
diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index 2683614d2e8..a452bbba422 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -140,6 +140,6 @@ class UsersController < ApplicationController
   end
 
   def projects_for_current_user
-    ProjectsFinder.new.execute(current_user)
+    ProjectsFinder.new(current_user: current_user).execute
   end
 end
diff --git a/app/finders/group_projects_finder.rb b/app/finders/group_projects_finder.rb
index 3b9a421b118..f043c38c6f9 100644
--- a/app/finders/group_projects_finder.rb
+++ b/app/finders/group_projects_finder.rb
@@ -1,42 +1,63 @@
-class GroupProjectsFinder < UnionFinder
-  def initialize(group, options = {})
+# GroupProjectsFinder
+#
+# Used to filter Projects  by set of params
+#
+# Arguments:
+#   current_user - which user use
+#   project_ids_relation: int[] - project ids to use
+#   group
+#   options:
+#     only_owned: boolean
+#     only_shared: boolean
+#   params:
+#     sort: string
+#     visibility_level: int
+#     tags: string[]
+#     personal: boolean
+#     search: string
+#     non_archived: boolean
+#
+class GroupProjectsFinder < ProjectsFinder
+  attr_reader :group, :options
+
+  def initialize(group:, params: {}, options: {}, current_user: nil, project_ids_relation: nil)
+    super(params: params, current_user: current_user, project_ids_relation: project_ids_relation)
     @group   = group
     @options = options
   end
 
-  def execute(current_user = nil)
-    segments = group_projects(current_user)
-    find_union(segments, Project)
-  end
-
   private
 
-  def group_projects(current_user)
-    only_owned  = @options.fetch(:only_owned, false)
-    only_shared = @options.fetch(:only_shared, false)
+  def init_collection
+    only_owned  = options.fetch(:only_owned, false)
+    only_shared = options.fetch(:only_shared, false)
 
     projects = []
 
     if current_user
-      if @group.users.include?(current_user)
-        projects << @group.projects unless only_shared
-        projects << @group.shared_projects unless only_owned
+      if group.users.include?(current_user)
+        projects << group.projects unless only_shared
+        projects << group.shared_projects unless only_owned
       else
         unless only_shared
-          projects << @group.projects.visible_to_user(current_user)
-          projects << @group.projects.public_to_user(current_user)
+          projects << group.projects.visible_to_user(current_user)
+          projects << group.projects.public_to_user(current_user)
         end
 
         unless only_owned
-          projects << @group.shared_projects.visible_to_user(current_user)
-          projects << @group.shared_projects.public_to_user(current_user)
+          projects << group.shared_projects.visible_to_user(current_user)
+          projects << group.shared_projects.public_to_user(current_user)
         end
       end
     else
-      projects << @group.projects.public_only unless only_shared
-      projects << @group.shared_projects.public_only unless only_owned
+      projects << group.projects.public_only unless only_shared
+      projects << group.shared_projects.public_only unless only_owned
     end
 
     projects
   end
+
+  def union(items)
+    find_union(items, Project)
+  end
 end
diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb
index f7ebb1807d7..4cc42b88a2a 100644
--- a/app/finders/issuable_finder.rb
+++ b/app/finders/issuable_finder.rb
@@ -116,9 +116,9 @@ class IssuableFinder
       if current_user && params[:authorized_only].presence && !current_user_related?
         current_user.authorized_projects
       elsif group
-        GroupProjectsFinder.new(group).execute(current_user)
+        GroupProjectsFinder.new(group: group, current_user: current_user).execute
       else
-        projects_finder.execute(current_user, item_project_ids(items))
+        ProjectsFinder.new(current_user: current_user, project_ids_relation: item_project_ids(items)).execute
       end
 
     @projects = projects.with_feature_available_for_user(klass, current_user).reorder(nil)
@@ -405,8 +405,4 @@ class IssuableFinder
   def current_user_related?
     params[:scope] == 'created-by-me' || params[:scope] == 'authored' || params[:scope] == 'assigned-to-me'
   end
-
-  def projects_finder
-    @projects_finder ||= ProjectsFinder.new
-  end
 end
diff --git a/app/finders/labels_finder.rb b/app/finders/labels_finder.rb
index e52083f86e4..042d792dada 100644
--- a/app/finders/labels_finder.rb
+++ b/app/finders/labels_finder.rb
@@ -83,7 +83,7 @@ class LabelsFinder < UnionFinder
   def projects
     return @projects if defined?(@projects)
 
-    @projects = skip_authorization ? Project.all : ProjectsFinder.new.execute(current_user)
+    @projects = skip_authorization ? Project.all : ProjectsFinder.new(current_user: current_user).execute
     @projects = @projects.in_namespace(params[:group_id]) if group?
     @projects = @projects.where(id: params[:project_ids]) if projects?
     @projects = @projects.reorder(nil)
diff --git a/app/finders/projects_finder.rb b/app/finders/projects_finder.rb
index 18ec45f300d..f6d8226bf3f 100644
--- a/app/finders/projects_finder.rb
+++ b/app/finders/projects_finder.rb
@@ -1,19 +1,93 @@
+# ProjectsFinder
+#
+# Used to filter Projects  by set of params
+#
+# Arguments:
+#   current_user - which user use
+#   project_ids_relation: int[] - project ids to use
+#   params:
+#     trending: boolean
+#     non_public: boolean
+#     starred: boolean
+#     sort: string
+#     visibility_level: int
+#     tags: string[]
+#     personal: boolean
+#     search: string
+#     non_archived: boolean
+#
 class ProjectsFinder < UnionFinder
-  def execute(current_user = nil, project_ids_relation = nil)
-    segments = all_projects(current_user)
-    segments.map! { |s| s.where(id: project_ids_relation) } if project_ids_relation
+  attr_accessor :params
+  attr_reader :current_user, :project_ids_relation
 
-    find_union(segments, Project).with_route
+  def initialize(params: {}, current_user: nil, project_ids_relation: nil)
+    @params = params
+    @current_user = current_user
+    @project_ids_relation = project_ids_relation
+  end
+
+  def execute
+    items = init_collection
+    items = by_ids(items)
+    items = union(items)
+    items = by_personal(items)
+    items = by_visibilty_level(items)
+    items = by_tags(items)
+    items = by_search(items)
+    items = by_archived(items)
+    sort(items)
   end
 
   private
 
-  def all_projects(current_user)
+  def init_collection
     projects = []
 
-    projects << current_user.authorized_projects if current_user
-    projects << Project.unscoped.public_to_user(current_user)
+    if params[:trending].present?
+      projects << Project.trending
+    elsif params[:starred].present? && current_user
+      projects << current_user.viewable_starred_projects
+    else
+      projects << current_user.authorized_projects if current_user
+      projects << Project.unscoped.public_to_user(current_user) unless params[:non_public].present?
+    end
 
     projects
   end
+
+  def by_ids(items)
+    project_ids_relation ? items.map { |item| item.where(id: project_ids_relation) } : items
+  end
+
+  def union(items)
+    find_union(items, Project).with_route
+  end
+
+  def by_personal(items)
+    (params[:personal].present? && current_user) ? items.personal(current_user) : items
+  end
+
+  def by_visibilty_level(items)
+    params[:visibility_level].present? ? items.where(visibility_level: params[:visibility_level]) : items
+  end
+
+  def by_tags(items)
+    params[:tag].present? ? items.tagged_with(params[:tag]) : items
+  end
+
+  def by_search(items)
+    params[:search] ||= params[:name]
+    params[:search].present? ? items.search(params[:search]) : items
+  end
+
+  def sort(items)
+    params[:sort].present? ? items.sort(params[:sort]) : items
+  end
+
+  def by_archived(projects)
+    # Back-compatibility with the places where `params[:archived]` can be set explicitly to `false`
+    params[:non_archived] = !Gitlab::Utils.to_boolean(params[:archived]) if params.key?(:archived)
+
+    params[:non_archived] ? projects.non_archived : projects
+  end
 end
diff --git a/app/finders/todos_finder.rb b/app/finders/todos_finder.rb
index b7f091f334d..dc13386184e 100644
--- a/app/finders/todos_finder.rb
+++ b/app/finders/todos_finder.rb
@@ -95,7 +95,7 @@ class TodosFinder
 
   def projects(items)
     item_project_ids = items.reorder(nil).select(:project_id)
-    ProjectsFinder.new.execute(current_user, item_project_ids)
+    ProjectsFinder.new(current_user: current_user, project_ids_relation: item_project_ids).execute
   end
 
   def type?
diff --git a/app/helpers/sorting_helper.rb b/app/helpers/sorting_helper.rb
index 5c89cbea3fc..3a5d1b97c36 100644
--- a/app/helpers/sorting_helper.rb
+++ b/app/helpers/sorting_helper.rb
@@ -25,8 +25,8 @@ module SortingHelper
   def projects_sort_options_hash
     options = {
       sort_value_name => sort_title_name,
-      sort_value_recently_updated => sort_title_recently_updated,
-      sort_value_oldest_updated => sort_title_oldest_updated,
+      sort_value_latest_activity => sort_title_latest_activity,
+      sort_value_oldest_activity => sort_title_oldest_activity,
       sort_value_recently_created => sort_title_recently_created,
       sort_value_oldest_created => sort_title_oldest_created
     }
@@ -78,6 +78,14 @@ module SortingHelper
     'Last updated'
   end
 
+  def sort_title_oldest_activity
+    'Oldest updated'
+  end
+
+  def sort_title_latest_activity
+    'Last updated'
+  end
+
   def sort_title_oldest_created
     'Oldest created'
   end
@@ -198,6 +206,14 @@ module SortingHelper
     'updated_desc'
   end
 
+  def sort_value_oldest_activity
+    'latest_activity_asc'
+  end
+
+  def sort_value_latest_activity
+    'latest_activity_desc'
+  end
+
   def sort_value_oldest_created
     'created_asc'
   end
diff --git a/app/models/project.rb b/app/models/project.rb
index 12fd0668ff8..1f95d00baf8 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -349,10 +349,15 @@ class Project < ActiveRecord::Base
     end
 
     def sort(method)
-      if method == 'storage_size_desc'
+      case method.to_s
+      when 'storage_size_desc'
         # storage_size is a joined column so we need to
         # pass a string to avoid AR adding the table name
         reorder('project_statistics.storage_size DESC, projects.id DESC')
+      when 'latest_activity_desc'
+        reorder(last_activity_at: :desc)
+      when 'latest_activity_asc'
+        reorder(last_activity_at: :asc)
       else
         order_by(method)
       end
diff --git a/app/policies/group_policy.rb b/app/policies/group_policy.rb
index 4cc21696eb6..cb58c115d54 100644
--- a/app/policies/group_policy.rb
+++ b/app/policies/group_policy.rb
@@ -12,7 +12,7 @@ class GroupPolicy < BasePolicy
     can_read ||= globally_viewable
     can_read ||= member
     can_read ||= @user.admin?
-    can_read ||= GroupProjectsFinder.new(@subject).execute(@user).any?
+    can_read ||= GroupProjectsFinder.new(group: @subject, current_user: @user).execute.any?
     can! :read_group if can_read
 
     # Only group masters and group owners can create new projects
@@ -41,6 +41,6 @@ class GroupPolicy < BasePolicy
     return true if @subject.internal? && !@user.external?
     return true if @subject.users.include?(@user)
 
-    GroupProjectsFinder.new(@subject).execute(@user).any?
+    GroupProjectsFinder.new(group: @subject, current_user: @user).execute.any?
   end
 end
diff --git a/app/services/search/global_service.rb b/app/services/search/global_service.rb
index c1549df5ac6..8409b592b72 100644
--- a/app/services/search/global_service.rb
+++ b/app/services/search/global_service.rb
@@ -8,7 +8,7 @@ module Search
 
     def execute
       group = Group.find_by(id: params[:group_id]) if params[:group_id].present?
-      projects = ProjectsFinder.new.execute(current_user)
+      projects = ProjectsFinder.new(current_user: current_user).execute
 
       if group
         projects = projects.inside_path(group.full_path)
diff --git a/app/views/dashboard/projects/_zero_authorized_projects.html.haml b/app/views/dashboard/projects/_zero_authorized_projects.html.haml
index 1bbd4602ecf..8843d4e7c84 100644
--- a/app/views/dashboard/projects/_zero_authorized_projects.html.haml
+++ b/app/views/dashboard/projects/_zero_authorized_projects.html.haml
@@ -1,4 +1,4 @@
-- publicish_project_count = ProjectsFinder.new.execute(current_user).count
+- publicish_project_count = ProjectsFinder.new(current_user: current_user).execute.count
 .blank-state.blank-state-welcome
   %h2.blank-state-welcome-title
     Welcome to GitLab
diff --git a/app/views/shared/projects/_dropdown.html.haml b/app/views/shared/projects/_dropdown.html.haml
index 2d25b8aad62..8939aeb6c3a 100644
--- a/app/views/shared/projects/_dropdown.html.haml
+++ b/app/views/shared/projects/_dropdown.html.haml
@@ -1,4 +1,4 @@
-- @sort ||= sort_value_recently_updated
+- @sort ||= sort_value_latest_activity
 .dropdown
   - toggle_text = projects_sort_options_hash[@sort]
   = dropdown_toggle(toggle_text, { toggle: 'dropdown' }, { id: 'sort-projects-dropdown' })
diff --git a/changelogs/unreleased/28810-projectfinder-should-handle-more-options.yml b/changelogs/unreleased/28810-projectfinder-should-handle-more-options.yml
new file mode 100644
index 00000000000..e4be16d4b37
--- /dev/null
+++ b/changelogs/unreleased/28810-projectfinder-should-handle-more-options.yml
@@ -0,0 +1,4 @@
+---
+title: ProjectsFinder should handle more options
+merge_request: 9682
+author: Jacopo Beschi @jacopo-beschi
diff --git a/lib/api/groups.rb b/lib/api/groups.rb
index 8f3799417e3..605769eddde 100644
--- a/lib/api/groups.rb
+++ b/lib/api/groups.rb
@@ -142,7 +142,7 @@ module API
       end
       get ":id/projects" do
         group = find_group!(params[:id])
-        projects = GroupProjectsFinder.new(group).execute(current_user)
+        projects = GroupProjectsFinder.new(group: group, current_user: current_user).execute
         projects = filter_projects(projects)
         entity = params[:simple] ? Entities::BasicProjectDetails : Entities::Project
         present paginate(projects), with: entity, current_user: current_user
diff --git a/lib/api/projects.rb b/lib/api/projects.rb
index 0fbe1669d45..766fbea53e6 100644
--- a/lib/api/projects.rb
+++ b/lib/api/projects.rb
@@ -84,7 +84,7 @@ module API
       end
       get do
         entity = current_user ? Entities::ProjectWithAccess : Entities::BasicProjectDetails
-        present_projects ProjectsFinder.new.execute(current_user), with: entity, statistics: params[:statistics]
+        present_projects ProjectsFinder.new(current_user: current_user).execute, with: entity, statistics: params[:statistics]
       end
 
       desc 'Create new project' do
diff --git a/lib/api/users.rb b/lib/api/users.rb
index 530ca0b5235..992a751b37d 100644
--- a/lib/api/users.rb
+++ b/lib/api/users.rb
@@ -341,7 +341,7 @@ module API
         not_found!('User') unless user
 
         events = user.events.
-          merge(ProjectsFinder.new.execute(current_user)).
+          merge(ProjectsFinder.new(current_user: current_user).execute).
           references(:project).
           with_associations.
           recent
diff --git a/lib/api/v3/groups.rb b/lib/api/v3/groups.rb
index c5b37622d79..9b27411ae21 100644
--- a/lib/api/v3/groups.rb
+++ b/lib/api/v3/groups.rb
@@ -151,7 +151,7 @@ module API
         end
         get ":id/projects" do
           group = find_group!(params[:id])
-          projects = GroupProjectsFinder.new(group).execute(current_user)
+          projects = GroupProjectsFinder.new(group: group, current_user: current_user).execute
           projects = filter_projects(projects)
           entity = params[:simple] ? ::API::Entities::BasicProjectDetails : Entities::Project
           present paginate(projects), with: entity, current_user: current_user
diff --git a/lib/api/v3/projects.rb b/lib/api/v3/projects.rb
index b753dbab381..ba9748ada59 100644
--- a/lib/api/v3/projects.rb
+++ b/lib/api/v3/projects.rb
@@ -107,7 +107,7 @@ module API
         end
         get '/visible' do
           entity = current_user ? ::API::V3::Entities::ProjectWithAccess : ::API::Entities::BasicProjectDetails
-          present_projects ProjectsFinder.new.execute(current_user), with: entity
+          present_projects ProjectsFinder.new(current_user: current_user).execute, with: entity
         end
 
         desc 'Get a projects list for authenticated user' do
diff --git a/lib/api/v3/users.rb b/lib/api/v3/users.rb
index 5e18cecc431..f4cda3b2eba 100644
--- a/lib/api/v3/users.rb
+++ b/lib/api/v3/users.rb
@@ -138,7 +138,7 @@ module API
           not_found!('User') unless user
 
           events = user.events.
-            merge(ProjectsFinder.new.execute(current_user)).
+            merge(ProjectsFinder.new(current_user: current_user).execute).
             references(:project).
             with_associations.
             recent
diff --git a/spec/finders/group_projects_finder_spec.rb b/spec/finders/group_projects_finder_spec.rb
index ef97b061ca7..3c7c9bdcd08 100644
--- a/spec/finders/group_projects_finder_spec.rb
+++ b/spec/finders/group_projects_finder_spec.rb
@@ -3,8 +3,9 @@ require 'spec_helper'
 describe GroupProjectsFinder do
   let(:group) { create(:group) }
   let(:current_user) { create(:user) }
+  let(:options) { {} }
 
-  let(:finder) { described_class.new(source_user) }
+  let(:finder) { described_class.new(group: group, current_user: current_user, options: options) }
 
   let!(:public_project) { create(:empty_project, :public, group: group, path: '1') }
   let!(:private_project) { create(:empty_project, :private, group: group, path: '2') }
@@ -18,22 +19,27 @@ describe GroupProjectsFinder do
     shared_project_3.project_group_links.create(group_access: Gitlab::Access::MASTER, group: group)
   end
 
+  subject { finder.execute }
+
   describe 'with a group member current user' do
-    before  { group.add_user(current_user, Gitlab::Access::MASTER) }
+    before do
+      group.add_master(current_user)
+    end
 
     context "only shared" do
-      subject { described_class.new(group, only_shared: true).execute(current_user) }
-      it      { is_expected.to eq([shared_project_3, shared_project_2, shared_project_1]) }
+      let(:options) { { only_shared: true } }
+
+      it { is_expected.to match_array([shared_project_3, shared_project_2, shared_project_1]) }
     end
 
     context "only owned" do
-      subject { described_class.new(group, only_owned: true).execute(current_user) }
-      it      { is_expected.to eq([private_project, public_project]) }
+      let(:options) { { only_owned: true } }
+
+      it { is_expected.to match_array([private_project, public_project]) }
     end
 
     context "all" do
-      subject { described_class.new(group).execute(current_user) }
-      it      { is_expected.to eq([shared_project_3, shared_project_2, shared_project_1, private_project, public_project]) }
+      it { is_expected.to match_array([shared_project_3, shared_project_2, shared_project_1, private_project, public_project]) }
     end
   end
 
@@ -44,47 +50,57 @@ describe GroupProjectsFinder do
     end
 
     context "only shared" do
+      let(:options) { { only_shared: true } }
+
       context "without external user" do
-        subject { described_class.new(group, only_shared: true).execute(current_user) }
-        it      { is_expected.to eq([shared_project_3, shared_project_2, shared_project_1]) }
+        it { is_expected.to match_array([shared_project_3, shared_project_2, shared_project_1]) }
       end
 
       context "with external user" do
-        before  { current_user.update_attributes(external: true) }
-        subject { described_class.new(group, only_shared: true).execute(current_user) }
-        it      { is_expected.to eq([shared_project_2, shared_project_1]) }
+        before do
+          current_user.update_attributes(external: true)
+        end
+
+        it { is_expected.to match_array([shared_project_2, shared_project_1]) }
       end
     end
 
     context "only owned" do
+      let(:options) { { only_owned: true } }
+
       context "without external user" do
-        before  { private_project.team << [current_user, Gitlab::Access::MASTER] }
-        subject { described_class.new(group, only_owned: true).execute(current_user) }
-        it      { is_expected.to eq([private_project, public_project]) }
+        before do
+          private_project.team << [current_user, Gitlab::Access::MASTER]
+        end
+
+        it { is_expected.to match_array([private_project, public_project]) }
       end
 
       context "with external user" do
-        before  { current_user.update_attributes(external: true) }
-        subject { described_class.new(group, only_owned: true).execute(current_user) }
-        it      { is_expected.to eq([public_project]) }
-      end
+        before do
+          current_user.update_attributes(external: true)
+        end
 
-      context "all" do
-        subject { described_class.new(group).execute(current_user) }
-        it      { is_expected.to eq([shared_project_3, shared_project_2, shared_project_1, public_project]) }
+        it { is_expected.to eq([public_project]) }
       end
     end
+
+    context "all" do
+      it { is_expected.to match_array([shared_project_3, shared_project_2, shared_project_1, public_project]) }
+    end
   end
 
   describe "no user" do
     context "only shared" do
-      subject { described_class.new(group, only_shared: true).execute(current_user) }
-      it      { is_expected.to eq([shared_project_3, shared_project_1]) }
+      let(:options) { { only_shared: true } }
+
+      it { is_expected.to match_array([shared_project_3, shared_project_1]) }
     end
 
     context "only owned" do
-      subject { described_class.new(group, only_owned: true).execute(current_user) }
-      it      { is_expected.to eq([public_project]) }
+      let(:options) { { only_owned: true } }
+
+      it { is_expected.to eq([public_project]) }
     end
   end
 end
diff --git a/spec/finders/projects_finder_spec.rb b/spec/finders/projects_finder_spec.rb
index e44e7434c80..148adcffe3b 100644
--- a/spec/finders/projects_finder_spec.rb
+++ b/spec/finders/projects_finder_spec.rb
@@ -21,38 +21,144 @@ describe ProjectsFinder do
       create(:empty_project, :private, name: 'D', path: 'D')
     end
 
-    let(:finder) { described_class.new }
+    let(:params) { {} }
+    let(:current_user) { user }
+    let(:project_ids_relation) { nil }
+    let(:finder) { described_class.new(params: params, current_user: current_user, project_ids_relation: project_ids_relation) }
+
+    subject { finder.execute }
 
     describe 'without a user' do
-      subject { finder.execute }
+      let(:current_user) { nil }
 
       it { is_expected.to eq([public_project]) }
     end
 
     describe 'with a user' do
-      subject { finder.execute(user) }
-
       describe 'without private projects' do
-        it { is_expected.to eq([public_project, internal_project]) }
+        it { is_expected.to match_array([public_project, internal_project]) }
       end
 
       describe 'with private projects' do
         before do
-          private_project.add_user(user, Gitlab::Access::MASTER)
+          private_project.add_master(user)
         end
 
-        it do
-          is_expected.to eq([public_project, internal_project, private_project])
-        end
+        it { is_expected.to match_array([public_project, internal_project, private_project]) }
       end
     end
 
     describe 'with project_ids_relation' do
       let(:project_ids_relation) { Project.where(id: internal_project.id) }
 
-      subject { finder.execute(user, project_ids_relation) }
-
       it { is_expected.to eq([internal_project]) }
     end
+
+    describe 'filter by visibility_level' do
+      before do
+        private_project.add_master(user)
+      end
+
+      context 'private' do
+        let(:params) { { visibility_level: Gitlab::VisibilityLevel::PRIVATE } }
+
+        it { is_expected.to eq([private_project]) }
+      end
+
+      context 'internal' do
+        let(:params) { { visibility_level: Gitlab::VisibilityLevel::INTERNAL } }
+
+        it { is_expected.to eq([internal_project]) }
+      end
+
+      context 'public' do
+        let(:params) { { visibility_level: Gitlab::VisibilityLevel::PUBLIC } }
+
+        it { is_expected.to eq([public_project]) }
+      end
+    end
+
+    describe 'filter by tags' do
+      before do
+        public_project.tag_list.add('foo')
+        public_project.save!
+      end
+
+      let(:params) { { tag: 'foo' } }
+
+      it { is_expected.to eq([public_project]) }
+    end
+
+    describe 'filter by personal' do
+      let!(:personal_project) { create(:empty_project, namespace: user.namespace) }
+      let(:params) { { personal: true } }
+
+      it { is_expected.to eq([personal_project]) }
+    end
+
+    describe 'filter by search' do
+      let(:params) { { search: 'C' } }
+
+      it { is_expected.to eq([public_project]) }
+    end
+
+    describe 'filter by name for backward compatibility' do
+      let(:params) { { name: 'C' } }
+
+      it { is_expected.to eq([public_project]) }
+    end
+
+    describe 'filter by archived' do
+      let!(:archived_project) { create(:empty_project, :public, :archived, name: 'E', path: 'E') }
+
+      context 'non_archived=true' do
+        let(:params) { { non_archived: true } }
+
+        it { is_expected.to match_array([public_project, internal_project]) }
+      end
+
+      context 'non_archived=false' do
+        let(:params) { { non_archived: false } }
+
+        it { is_expected.to match_array([public_project, internal_project, archived_project]) }
+      end
+
+      describe 'filter by archived for backward compatibility' do
+        let(:params) { { archived: false } }
+
+        it { is_expected.to match_array([public_project, internal_project]) }
+      end
+    end
+
+    describe 'filter by trending' do
+      let!(:trending_project) { create(:trending_project, project: public_project)  }
+      let(:params) { { trending: true } }
+
+      it { is_expected.to eq([public_project]) }
+    end
+
+    describe 'filter by non_public' do
+      let(:params) { { non_public: true } }
+      before do
+        private_project.add_developer(current_user)
+      end
+
+      it { is_expected.to eq([private_project]) }
+    end
+
+    describe 'filter by viewable_starred_projects' do
+      let(:params) { { starred: true } }
+      before do
+        current_user.toggle_star(public_project)
+      end
+
+      it { is_expected.to eq([public_project]) }
+    end
+
+    describe 'sorting' do
+      let(:params) { { sort: 'name_asc' } }
+
+      it { is_expected.to eq([internal_project, public_project]) }
+    end
   end
 end
-- 
cgit v1.2.3