This instance will be upgraded to Heptapod 0.26.0rc1 on 2021-10-26 at 14:30 UTC+2 (a few minutes of down time)

Commit 0a11fb5e authored by Yorick Peterse's avatar Yorick Peterse
Browse files

Refactor AutocompleteController

This refactors the AutocompleteController according to the guidelines
and boundaries discussed in
https://gitlab.com/gitlab-org/gitlab-ce/issues/49653. Specifically,
ActiveRecord logic is moved to different finders, which are then used in
the controller. View logic in turn is moved to presenters, instead of
directly using ActiveRecord's "to_json" method.

The finder MoveToProjectFinder is also adjusted according to the
abstraction guidelines and boundaries, resulting in a much more simple
finder.

By using finders (and other abstractions) more actively, we can push a
lot of logic out of the controller. We also remove the need for various
"before_action" hooks, though this could be achieved without using
finders as well.

The various finders related to AutcompleteController have also been
moved into a namespace. This removes the need for calling everything
"AutocompleteSmurfFinder", instead you can use
"Autocomplete::SmurfFinder".
parent 82e260236ce7
class AutocompleteController < ApplicationController
AWARD_EMOJI_MAX = 100
skip_before_action :authenticate_user!, only: [:users, :award_emojis]
before_action :load_project, only: [:users]
before_action :load_group, only: [:users]
def users
@users = AutocompleteUsersFinder.new(params: params, current_user: current_user, project: @project, group: @group).execute
render json: UserSerializer.new.represent(@users)
end
def user
@user = User.find(params[:id])
render json: UserSerializer.new.represent(@user)
end
def projects
project = Project.find_by_id(params[:project_id])
projects = projects_finder.execute(project, search: params[:search], offset_id: params[:offset_id])
project = Autocomplete::ProjectFinder
.new(current_user, params)
.execute
render json: projects.to_json(only: [:id, :name_with_namespace], methods: :name_with_namespace)
end
group = Autocomplete::GroupFinder
.new(current_user, project, params)
.execute
def award_emojis
emoji_with_count = AwardEmoji
.limit(AWARD_EMOJI_MAX)
.where(user: current_user)
.group(:name)
.order('count_all DESC, name ASC')
.count
users = Autocomplete::UsersFinder
.new(params: params, current_user: current_user, project: project, group: group)
.execute
# Transform from hash to array to guarantee json order
# e.g. { 'thumbsup' => 2, 'thumbsdown' = 1 }
# => [{ name: 'thumbsup' }, { name: 'thumbsdown' }]
render json: emoji_with_count.map { |k, v| { name: k } }
render json: UserSerializer.new.represent(users)
end
private
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)
def user
user = UserFinder.new(params).execute!
group
end
end
render json: UserSerializer.new.represent(user)
end
def load_project
@project ||= begin
if params[:project_id].present?
project = Project.find(params[:project_id])
return render_404 unless can?(current_user, :read_project, project)
# Displays projects to use for the dropdown when moving a resource from one
# project to another.
def projects
projects = Autocomplete::MoveToProjectFinder
.new(current_user, params)
.execute
project
end
end
render json: MoveToProjectSerializer.new.represent(projects)
end
def projects_finder
MoveToProjectFinder.new(current_user)
def award_emojis
render json: AwardedEmojiFinder.new(current_user).execute
end
end
# frozen_string_literal: true
module Autocomplete
# Finder for retrieving a group to use for autocomplete data sources.
class GroupFinder
attr_reader :current_user, :project, :group_id
# current_user - The currently logged in user, if any.
# project - The Project (if any) to use for the autocomplete data sources.
# params - A Hash containing parameters to use for finding the project.
#
# The following parameters are supported:
#
# * group_id: The ID of the group to find.
def initialize(current_user = nil, project = nil, params = {})
@current_user = current_user
@project = project
@group_id = params[:group_id]
end
# Attempts to find a Group based on the current group ID.
def execute
return unless project.blank? && group_id.present?
group = Group.find(group_id)
# This removes the need for using `return render_404` and similar patterns
# in controllers that use this finder.
unless Ability.allowed?(current_user, :read_group, group)
raise ActiveRecord::RecordNotFound
.new("Could not find a Group with ID #{group_id}")
end
group
end
end
end
# frozen_string_literal: true
module Autocomplete
# Finder that retrieves a list of projects that an issue can be moved to.
class MoveToProjectFinder
attr_reader :current_user, :search, :project_id, :offset_id
# current_user - The User object of the user that wants to view the list of
# projects.
#
# params - A Hash containing additional parameters to set.
#
# The following parameters can be set (as Symbols):
#
# * search: An optional search query to apply to the list of projects.
# * project_id: The ID of a project to exclude from the returned relation.
# * offset_id: The ID of a project to use for pagination. When given, only
# projects with a lower ID are included in the list.
def initialize(current_user, params = {})
@current_user = current_user
@search = params[:search]
@project_id = params[:project_id]
@offset_id = params[:offset_id]
end
def execute
current_user
.projects_where_can_admin_issues
.optionally_search(search)
.excluding_project(project_id)
.paginate_in_descending_order_using_id(before: offset_id)
.eager_load_namespace_and_owner
end
end
end
# frozen_string_literal: true
module Autocomplete
# Finder for retrieving a project to use for autocomplete data sources.
class ProjectFinder
attr_reader :current_user, :project_id
# current_user - The currently logged in user, if any.
# params - A Hash containing parameters to use for finding the project.
#
# The following parameters are supported:
#
# * project_id: The ID of the project to find.
def initialize(current_user = nil, params = {})
@current_user = current_user
@project_id = params[:project_id]
end
# Attempts to find a Project based on the current project ID.
def execute
return if project_id.blank?
project = Project.find(project_id)
# This removes the need for using `return render_404` and similar patterns
# in controllers that use this finder.
unless Ability.allowed?(current_user, :read_project, project)
raise ActiveRecord::RecordNotFound
.new("Could not find a Project with ID #{project_id}")
end
project
end
end
end
# frozen_string_literal: true
module Autocomplete
class UsersFinder
# The number of users to display in the results is hardcoded to 20, and
# pagination is not supported. This ensures that performance remains
# consistent and removes the need for implementing keyset pagination to
# ensure good performance.
LIMIT = 20
attr_reader :current_user, :project, :group, :search, :skip_users,
:author_id, :todo_filter, :todo_state_filter,
:filter_by_current_user
def initialize(params:, current_user:, project:, group:)
@current_user = current_user
@project = project
@group = group
@search = params[:search]
@skip_users = params[:skip_users]
@author_id = params[:author_id]
@todo_filter = params[:todo_filter]
@todo_state_filter = params[:todo_state_filter]
@filter_by_current_user = params[:current_user]
end
def execute
items = limited_users
if search.blank?
# Include current user if available to filter by "Me"
items.unshift(current_user) if prepend_current_user?
if prepend_author? && (author = User.find_by_id(author_id))
items.unshift(author)
end
end
items.uniq
end
private
# Returns the users based on the input parameters, as an Array.
#
# This method is separate so it is easier to extend in EE.
def limited_users
# When changing the order of these method calls, make sure that
# reorder_by_name() is called _before_ optionally_search(), otherwise
# reorder_by_name will break the ORDER BY applied in optionally_search().
find_users
.active
.reorder_by_name
.optionally_search(search)
.where_not_in(skip_users)
.limit_to_todo_authors(
user: current_user,
with_todos: todo_filter,
todo_state: todo_state_filter
)
.limit(LIMIT)
.to_a
end
def prepend_current_user?
filter_by_current_user.present? && current_user
end
def prepend_author?
author_id.present? && current_user
end
def find_users
if project
project.authorized_users.union_with_user(author_id)
elsif group
group.users_with_parents
elsif current_user
User.all
else
User.none
end
end
end
end
class AutocompleteUsersFinder
# The number of users to display in the results is hardcoded to 20, and
# pagination is not supported. This ensures that performance remains
# consistent and removes the need for implementing keyset pagination to ensure
# good performance.
LIMIT = 20
attr_reader :current_user, :project, :group, :search, :skip_users,
: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]
@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.limit(LIMIT)
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_with_parents if group
return User.all if current_user
User.none
end
def users_from_project
if author_id.present?
union = Gitlab::SQL::Union
.new([project.authorized_users, User.where(id: author_id)])
User.from("(#{union.to_sql}) #{User.table_name}")
else
project.authorized_users
end
end
end
# frozen_string_literal: true
# Class for retrieving information about emoji awarded _by_ a particular user.
class AwardedEmojiFinder
attr_reader :current_user
# current_user - The User to generate the data for.
def initialize(current_user = nil)
@current_user = current_user
end
def execute
return [] unless current_user
# We want the resulting data set to be an Array containing the emoji names
# in descending order, based on how often they were awarded.
AwardEmoji
.award_counts_for_user(current_user)
.map { |name, _| { name: name } }
end
end
class MoveToProjectFinder
PAGE_SIZE = 50
def initialize(user)
@user = user
end
def execute(from_project, search: nil, offset_id: nil)
projects = @user.projects_where_can_admin_issues
projects = projects.search(search) if search.present?
projects = projects.excluding_project(from_project)
projects = projects.order_id_desc
# infinite scroll using offset
projects = projects.where('projects.id < ?', offset_id) if offset_id.present?
projects = projects.limit(PAGE_SIZE)
# to ask for Project#name_with_namespace
projects.includes(namespace: :owner)
end
end
# frozen_string_literal: true
# A simple finding for obtaining a single User.
#
# While using `User.find_by` directly is straightforward, it can lead to a lot
# of code duplication. Sometimes we just want to find a user by an ID, other
# 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
def initialize(params)
@params = params
end
# Tries to find a User, returning nil if none could be found.
def execute
User.find_by(id: params[:id])
end
# Tries to find a User, raising a `ActiveRecord::RecordNotFound` if it could
# not be found.
def execute!
User.find(params[:id])
end
end
......@@ -28,6 +28,23 @@ def votes_for_collection(ids, type)
.where('name IN (?) AND awardable_type = ? AND awardable_id IN (?)', [DOWNVOTE_NAME, UPVOTE_NAME], type, ids)
.group('name', 'awardable_id')
end
# Returns the top 100 emoji awarded by the given user.
#
# The returned value is a Hash mapping emoji names to the number of times
# they were awarded:
#
# { 'thumbsup' => 2, 'thumbsdown' => 1 }
#
# user - The User to get the awards for.
# limt - The maximum number of emoji to return.
def award_counts_for_user(user, limit = 100)
limit(limit)
.where(user: user)
.group(:name)
.order('count_all DESC, name ASC')
.count
end
end
def downvote?
......
# frozen_string_literal: true
module OptionallySearch
extend ActiveSupport::Concern
module ClassMethods
def search(*)
raise(
NotImplementedError,
'Your model must implement the "search" class method'
)
end
# Optionally limits a result set to those matching the given search query.
def optionally_search(query = nil)
query.present? ? search(query) : all
end
end
end
......@@ -28,6 +28,7 @@ class Project < ActiveRecord::Base
include WithUploads
include BatchDestroyDependentAssociations
include FeatureGate
include OptionallySearch
extend Gitlab::Cache::RequestCache
extend Gitlab::ConfigHelper
......@@ -384,6 +385,26 @@ class Project < ActiveRecord::Base
only_integer: true,
message: 'needs to be beetween 10 minutes and 1 month' }
# Paginates a collection using a `WHERE id < ?` condition.
#
# before - A project ID to use for filtering out projects with an equal or
# greater ID. If no ID is given, all projects are included.
#
# limit - The maximum number of rows to include.
def self.paginate_in_descending_order_using_id(
before: nil,
limit: Kaminari.config.default_per_page
)
relation = order_id_desc.limit(limit)
relation = relation.where('projects.id < ?', before) if before
relation
end
def self.eager_load_namespace_and_owner
includes(namespace: :owner)
end
# Returns a collection of projects that is either public or visible to the
# logged in user.
def self.public_or_visible_to_user(user = nil)
......
......@@ -19,6 +19,7 @@ class User < ActiveRecord::Base
include BulkMemberAccessLoad
include BlocksJsonSerialization
include WithUploads
include OptionallySearch
DEFAULT_NOTIFICATION_LEVEL = :participating
......@@ -253,11 +254,41 @@ def inactive_message
scope :external, -> { where(external: true) }
scope :active, -> { with_state(:active).non_internal }
scope :without_projects, -> { joins('LEFT JOIN project_authorizations ON users.id = project_authorizations.user_id').where(project_authorizations: { user_id: nil }) }
scope :todo_authors, ->(user_id, state) { where(id: Todo.where(user_id: user_id, state: state).select(:author_id)) }
scope :order_recent_sign_in, -> { reorder(Gitlab::Database.nulls_last_order('current_sign_in_at', 'DESC')) }
scope :order_oldest_sign_in, -> { reorder(Gitlab::Database.nulls_last_order('current_sign_in_at', 'ASC')) }
scope :confirmed, -> { where.not(confirmed_at: nil) }
# Limits the users to those that have TODOs, optionally in the given state.
#
# user - The user to get the todos for.
#
# with_todos - If we should limit the result set to users that are the
# authors of todos.
#
# todo_state - An optional state to require the todos to be in.
def self.limit_to_todo_authors(user: nil, with_todos: false, todo_state: nil)
if user && with_todos
where(id: Todo.where(user: user, state: todo_state).select(:author_id))
else
all
end
end
# Returns a relation that optionally includes the given user.
#
# user_id - The ID of the user to include.
def self.union_with_user(user_id = nil)
if user_id.present?
union = Gitlab::SQL::Union.new([all, User.unscoped.where(id: user_id)])
# We use "unscoped" here so that any inner conditions are not repeated for
# the outer query, which would be redundant.
User.unscoped.from("(#{union.to_sql}) #{User.table_name}")
else
all
end
end
def self.with_two_factor_indistinct
joins("LEFT OUTER JOIN u2f_registrations AS u2f ON u2f.user_id = users.id")
.where("u2f.id IS NOT NULL OR users.otp_required_for_login = ?", true)
......@@ -365,6 +396,18 @@ def search(query)
).reorder(order % { query: ActiveRecord::Base.connection.quote(query) }, :name)
end
# Limits the result set to users _not_ in the given query/list of IDs.
#
# users - The list of users to ignore. This can be an
# `ActiveRecord::Relation`, or an Array.
def where_not_in(users = nil)
users ? where.not(id: users) : all
end
def reorder_by_name
reorder(:name)
end
# searches user by given pattern
# it compares name, email, username fields and user's secondary emails with given pattern
# This method uses ILIKE on PostgreSQL and LIKE on MySQL.
......
# frozen_string_literal: true
class MoveToProjectEntity < Grape::Entity
expose :id
expose :name_with_namespace
end
# frozen_string_literal: true
class MoveToProjectSerializer < BaseSerializer
entity MoveToProjectEntity
end
......@@ -13,7 +13,7 @@ def initialize(note, project, client)
@note = note
@project = project
@client = client
@user_finder = UserFinder.new(project, client)
@user_finder = GithubImport::UserFinder.new(project, client)
end
def execute
......
......@@ -19,7 +19,7 @@ def initialize(issue, project, client)
@issue = issue
@project = project
@client = client