summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGrzegorz Bizon <grzegorz@gitlab.com>2017-09-18 08:56:20 +0000
committerGrzegorz Bizon <grzegorz@gitlab.com>2017-09-18 08:56:20 +0000
commitfc504b887af54c62b247201282c14ccb45d9f479 (patch)
tree17816fe28e782a08206c369b567e69d283079aec
parentfffae7a0bfeb0eea9167bb87c3ecd1d557cb327a (diff)
parent27df56ad129ec145754ef16cc2c670850b7a5101 (diff)
downloadgitlab-ce-fc504b887af54c62b247201282c14ccb45d9f479.tar.gz
Merge branch '24121_extract_yet_another_users_finder' into 'master'
Extract AutocompleteController#users into finder Closes #24121 See merge request gitlab-org/gitlab-ce!13778
-rw-r--r--app/controllers/autocomplete_controller.rb47
-rw-r--r--app/finders/autocomplete_users_finder.rb60
-rw-r--r--changelogs/unreleased/24121_extract_yet_another_users_finder.yml5
-rw-r--r--spec/finders/autocomplete_users_finder_spec.rb97
4 files changed, 169 insertions, 40 deletions
diff --git a/app/controllers/autocomplete_controller.rb b/app/controllers/autocomplete_controller.rb
index dfc8bd0ba81..10e8e54f402 100644
--- a/app/controllers/autocomplete_controller.rb
+++ b/app/controllers/autocomplete_controller.rb
@@ -3,31 +3,10 @@ class AutocompleteController < ApplicationController
skip_before_action :authenticate_user!, only: [:users, :award_emojis]
before_action :load_project, only: [:users]
- before_action :find_users, only: [:users]
+ before_action :load_group, only: [:users]
def users
- @users ||= User.none
- @users = @users.active
- @users = @users.reorder(:name)
- @users = @users.search(params[:search]) if params[:search].present?
- @users = @users.where.not(id: params[:skip_users]) if params[:skip_users].present?
- @users = @users.page(params[:page]).per(params[:per_page])
-
- if params[:todo_filter].present? && current_user
- @users = @users.todo_authors(current_user.id, params[:todo_state_filter])
- end
-
- if params[:search].blank?
- # Include current user if available to filter by "Me"
- if params[:current_user].present? && current_user
- @users = [current_user, *@users].uniq
- end
-
- if params[:author_id].present? && current_user
- author = User.find_by_id(params[:author_id])
- @users = [author, *@users].uniq if author
- end
- end
+ @users = AutocompleteUsersFinder.new(params: params, current_user: current_user, project: @project, group: @group).execute
render json: @users, only: [:name, :username, :id], methods: [:avatar_url]
end
@@ -60,26 +39,14 @@ class AutocompleteController < ApplicationController
private
- def find_users
- @users =
- if @project
- user_ids = @project.team.users.pluck(:id)
-
- if params[:author_id].present?
- user_ids << params[:author_id]
- end
-
- User.where(id: user_ids)
- elsif params[:group_id].present?
+ def load_group
+ @group ||= begin
+ if @project.blank? && params[:group_id].present?
group = Group.find(params[:group_id])
return render_404 unless can?(current_user, :read_group, group)
-
- group.users
- elsif current_user
- User.all
- else
- User.none
+ group
end
+ end
end
def load_project
diff --git a/app/finders/autocomplete_users_finder.rb b/app/finders/autocomplete_users_finder.rb
new file mode 100644
index 00000000000..b8f52e31926
--- /dev/null
+++ b/app/finders/autocomplete_users_finder.rb
@@ -0,0 +1,60 @@
+class AutocompleteUsersFinder
+ attr_reader :current_user, :project, :group, :search, :skip_users,
+ :page, :per_page, :author_id, :params
+
+ def initialize(params:, current_user:, project:, group:)
+ @current_user = current_user
+ @project = project
+ @group = group
+ @search = params[:search]
+ @skip_users = params[:skip_users]
+ @page = params[:page]
+ @per_page = params[:per_page]
+ @author_id = params[:author_id]
+ @params = params
+ end
+
+ def execute
+ items = find_users
+ items = items.active
+ items = items.reorder(:name)
+ items = items.search(search) if search.present?
+ items = items.where.not(id: skip_users) if skip_users.present?
+ items = items.page(page).per(per_page)
+
+ if params[:todo_filter].present? && current_user
+ items = items.todo_authors(current_user.id, params[:todo_state_filter])
+ end
+
+ if search.blank?
+ # Include current user if available to filter by "Me"
+ if params[:current_user].present? && current_user
+ items = [current_user, *items].uniq
+ end
+
+ if author_id.present? && current_user
+ author = User.find_by_id(author_id)
+ items = [author, *items].uniq if author
+ end
+ end
+
+ items
+ end
+
+ private
+
+ def find_users
+ return users_from_project if project
+ return group.users if group
+ return User.all if current_user
+
+ User.none
+ end
+
+ def users_from_project
+ user_ids = project.team.users.pluck(:id)
+ user_ids << author_id if author_id.present?
+
+ User.where(id: user_ids)
+ end
+end
diff --git a/changelogs/unreleased/24121_extract_yet_another_users_finder.yml b/changelogs/unreleased/24121_extract_yet_another_users_finder.yml
new file mode 100644
index 00000000000..e43e97303e2
--- /dev/null
+++ b/changelogs/unreleased/24121_extract_yet_another_users_finder.yml
@@ -0,0 +1,5 @@
+---
+title: Extract AutocompleteController#users into finder
+merge_request: 13778
+author: Maxim Rydkin, Mayra Cabrera
+type: other
diff --git a/spec/finders/autocomplete_users_finder_spec.rb b/spec/finders/autocomplete_users_finder_spec.rb
new file mode 100644
index 00000000000..684af74d750
--- /dev/null
+++ b/spec/finders/autocomplete_users_finder_spec.rb
@@ -0,0 +1,97 @@
+require 'spec_helper'
+
+describe AutocompleteUsersFinder do
+ describe '#execute' do
+ let!(:user1) { create(:user, username: 'johndoe') }
+ let!(:user2) { create(:user, :blocked, username: 'notsorandom') }
+ let!(:external_user) { create(:user, :external) }
+ let!(:omniauth_user) { create(:omniauth_user, provider: 'twitter', extern_uid: '123456') }
+ let(:current_user) { create(:user) }
+ let(:params) { {} }
+
+ let(:project) { nil }
+ let(:group) { nil }
+
+ subject { described_class.new(params: params, current_user: current_user, project: project, group: group).execute.to_a }
+
+ context 'when current_user not passed or nil' do
+ let(:current_user) { nil }
+
+ it { is_expected.to match_array([]) }
+ end
+
+ context 'when project passed' do
+ let(:project) { create(:project) }
+
+ it { is_expected.to match_array([project.owner]) }
+
+ context 'when author_id passed' do
+ let(:params) { { author_id: user2.id } }
+
+ it { is_expected.to match_array([project.owner, user2]) }
+ end
+ end
+
+ context 'when group passed and project not passed' do
+ let(:group) { create(:group, :public) }
+
+ before do
+ group.add_users([user1], GroupMember::DEVELOPER)
+ end
+
+ it { is_expected.to match_array([user1]) }
+ end
+
+ it { is_expected.to match_array([user1, external_user, omniauth_user, current_user]) }
+
+ context 'when filtered by search' do
+ let(:params) { { search: 'johndoe' } }
+
+ it { is_expected.to match_array([user1]) }
+ end
+
+ context 'when filtered by skip_users' do
+ let(:params) { { skip_users: [omniauth_user.id, current_user.id] } }
+
+ it { is_expected.to match_array([user1, external_user]) }
+ end
+
+ context 'when todos exist' do
+ let!(:pending_todo1) { create(:todo, user: current_user, author: user1, state: :pending) }
+ let!(:pending_todo2) { create(:todo, user: external_user, author: omniauth_user, state: :pending) }
+ let!(:done_todo1) { create(:todo, user: current_user, author: external_user, state: :done) }
+ let!(:done_todo2) { create(:todo, user: user1, author: external_user, state: :done) }
+
+ context 'when filtered by todo_filter without todo_state_filter' do
+ let(:params) { { todo_filter: true } }
+
+ it { is_expected.to match_array([]) }
+ end
+
+ context 'when filtered by todo_filter with pending todo_state_filter' do
+ let(:params) { { todo_filter: true, todo_state_filter: 'pending' } }
+
+ it { is_expected.to match_array([user1]) }
+ end
+
+ context 'when filtered by todo_filter with done todo_state_filter' do
+ let(:params) { { todo_filter: true, todo_state_filter: 'done' } }
+
+ it { is_expected.to match_array([external_user]) }
+ end
+ end
+
+ context 'when filtered by current_user' do
+ let(:current_user) { user2 }
+ let(:params) { { current_user: true } }
+
+ it { is_expected.to match_array([user2, user1, external_user, omniauth_user]) }
+ end
+
+ context 'when filtered by author_id' do
+ let(:params) { { author_id: user2.id } }
+
+ it { is_expected.to match_array([user2, user1, external_user, omniauth_user, current_user]) }
+ end
+ end
+end