Skip to content
Snippets Groups Projects
Commit 25ec327a authored by William George's avatar William George
Browse files

Update UserFinder to accept `username` or `id`

Update code to use the new `UserFinder` as opposed to `User.find_by_username`
parent 0c3ad6b6
No related branches found
No related tags found
No related merge requests found
Pipeline #30437994 failed
......@@ -18,7 +18,7 @@ class AutocompleteController < ApplicationController
end
def user
user = UserFinder.new(params).execute!
user = UserFinder.new(params[:id]).execute!
render json: UserSerializer.new.represent(user)
end
......
......@@ -36,7 +36,7 @@ class Profiles::KeysController < Profiles::ApplicationController
def get_keys
if params[:username].present?
begin
user = User.find_by_username(params[:username])
user = UserFinder.new(params[:username]).find_by_username
if user.present?
render text: user.all_ssh_keys.join("\n"), content_type: "text/plain"
else
......
......@@ -26,9 +26,7 @@ class SnippetsController < ApplicationController
def index
if params[:username].present?
@user = User.find_by_username(params[:username])
return render_404 unless @user
@user = UserFinder.new(params[:username]).find_by_username!
@snippets = SnippetsFinder.new(current_user, author: @user, scope: params[:scope])
.execute.page(params[:page])
......
......@@ -254,7 +254,7 @@ class IssuableFinder
if assignee_id?
User.find_by(id: params[:assignee_id])
elsif assignee_username?
User.find_by_username(params[:assignee_username])
UserFinder.new(params[:assignee_username]).find_by_username
else
nil
end
......@@ -282,7 +282,7 @@ class IssuableFinder
if author_id?
User.find_by(id: params[:author_id])
elsif author_username?
User.find_by_username(params[:author_username])
UserFinder.new(params[:author_username]).find_by_username
else
nil
end
......
......@@ -7,22 +7,53 @@
# times we may want to exclude blocked user. By using this finder (and extending
# it whenever necessary) we can keep this logic in one place.
class UserFinder
attr_reader :params
attr_reader :username_or_id,
def initialize(params)
@params = params
def initialize(username_or_id)
@username_or_id = username_or_id
end
# Tries to find a User, returning nil if none could be found.
# Tries to find a User by id, returning nil if none could be found.
# rubocop: disable CodeReuse/ActiveRecord
def execute
User.find_by(id: params[:id])
def find_by_id
User.find_by_id(@username_or_id)
end
# rubocop: enable CodeReuse/ActiveRecord
# Tries to find a User, raising a `ActiveRecord::RecordNotFound` if it could
# Tries to find a User by id, raising a `ActiveRecord::RecordNotFound` if it could
# not be found.
def find_by_id!
User.find(@username_or_id)
end
# Tries to find a User by username, returning nil if none could be found.
def find_by_username
User.find_by_username(@username_or_id)
end
# Tries to find a User by username, raising a `ActiveRecord::RecordNotFound` if it could
# not be found.
def find_by_username!
User.find_by_username!(@username_or_id)
end
# Tries to find a User by username or id, returning nil if none could be found.
def execute
if @username_or_id =~ /^\d+$/
find_by_id
else
find_by_username
end
end
# Tries to find a User by username or id, raising a `ActiveRecord::RecordNotFound` if it could
# not be found.
def execute!
User.find(params[:id])
if @username_or_id =~ /^\d+$/
find_by_id!
else
find_by_username!
end
end
end
......@@ -48,7 +48,7 @@ require File.expand_path('../config/environment', File.dirname(__FILE__))
result = Gitlab::Profiler.profile(options[:url],
logger: Logger.new(options[:sql_output]),
post_data: options[:post_data],
user: User.find_by_username(options[:username]),
user: UserFinder.new(options[:username]).find_by_username,
private_token: ENV['PRIVATE_TOKEN'])
printer = RubyProf::CallStackPrinter.new(result)
......
......@@ -18,7 +18,7 @@ module API
def gate_targets(params)
targets = []
targets << Feature.group(params[:feature_group]) if params[:feature_group]
targets << User.find_by_username(params[:user]) if params[:user]
targets << UserFinder.new(params[:user]).find_by_username if params[:user]
targets
end
......
......@@ -94,15 +94,9 @@ module API
LabelsFinder.new(current_user, search_params).execute
end
# rubocop: disable CodeReuse/ActiveRecord
def find_user(id)
if id =~ /^\d+$/
User.find_by(id: id)
else
User.find_by_username(id)
end
UserFinder.new(id).execute
end
# rubocop: enable CodeReuse/ActiveRecord
# rubocop: disable CodeReuse/ActiveRecord
def find_project(id)
......
......@@ -38,7 +38,7 @@ module API
elsif params[:user_id]
User.find_by(id: params[:user_id])
elsif params[:username]
User.find_by_username(params[:username])
UserFinder.new(params[:username]).find_by_username
end
protocol = params[:protocol]
......@@ -152,7 +152,7 @@ module API
elsif params[:user_id]
user = User.find_by(id: params[:user_id])
elsif params[:username]
user = User.find_by_username(params[:username])
user = UserFinder.new(params[:username]).find_by_username
end
present user, with: Entities::UserSafe
......
......@@ -102,7 +102,7 @@ module Gitlab
if username.start_with?("@")
username = username[1..-1]
if user = User.find_by_username(username)
if user = UserFinder.new(username).find_by_username
assignee_id = user.id
end
end
......
......@@ -86,7 +86,7 @@ module Gitlab
# Example:
#
# Gitlab::Metrics.measure(:find_by_username_duration) do
# User.find_by_username(some_username)
# UserFinder.new(some_username).find_by_username
# end
#
# name - The name of the field to store the execution time in.
......
......@@ -9,7 +9,7 @@ class GithubImport
def initialize(token, gitlab_username, project_path, extras)
@options = { token: token }
@project_path = project_path
@current_user = User.find_by_username(gitlab_username)
@current_user = UserFinder.new(gitlab_username).find_by_username
raise "GitLab user #{gitlab_username} not found. Please specify a valid username." unless @current_user
......
......@@ -4,10 +4,19 @@ require 'spec_helper'
describe UserFinder do
describe '#execute' do
context 'when the user exists' do
context 'when the user exists (id)' do
it 'returns the user' do
user = create(:user)
found = described_class.new(id: user.id).execute
found = described_class.new(user.id).execute
expect(found).to eq(user)
end
end
context 'when the user exists (username)' do
it 'returns the user' do
user = create(:user)
found = described_class.new(user.username).execute
expect(found).to eq(user)
end
......@@ -15,7 +24,7 @@ describe UserFinder do
context 'when the user does not exist' do
it 'returns nil' do
found = described_class.new(id: 1).execute
found = described_class.new(1).execute
expect(found).to be_nil
end
......@@ -23,10 +32,19 @@ describe UserFinder do
end
describe '#execute!' do
context 'when the user exists' do
context 'when the user exists (id)' do
it 'returns the user' do
user = create(:user)
found = described_class.new(user.id).execute!
expect(found).to eq(user)
end
end
context 'when the user exists (username)' do
it 'returns the user' do
user = create(:user)
found = described_class.new(id: user.id).execute!
found = described_class.new(user.username).execute!
expect(found).to eq(user)
end
......@@ -34,7 +52,7 @@ describe UserFinder do
context 'when the user does not exist' do
it 'raises ActiveRecord::RecordNotFound' do
finder = described_class.new(id: 1)
finder = described_class.new(1)
expect { finder.execute! }.to raise_error(ActiveRecord::RecordNotFound)
end
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment