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: [PATCH] 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 +
.../dashboard/projects_controller.rb | 27 ++--
.../explore/projects_controller.rb | 34 ++---
.../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 +-
.../_zero_authorized_projects.html.haml | 2 +-
app/views/shared/projects/_dropdown.html.haml | 2 +-
...ojectfinder-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
--
GitLab