Read about our upcoming Code of Conduct on this issue

Commit 69ed8cbf authored by Francisco Lopez's avatar Francisco Lopez
Browse files

Comments from code review applied. Also switched forked_from_project and...

Comments from code review applied. Also switched forked_from_project and ForkedProjectLinks to ForkNetworkMember
parent 0c7857bc0691
......@@ -289,6 +289,14 @@ def full_path_was
"#{parent.full_path}/#{path_was}"
end
def group_member(user)
if group_members.loaded?
group_members.find { |gm| gm.user_id == user.id }
else
group_members.find_by(user_id: user)
end
end
private
def update_two_factor_requirement
......
......@@ -1114,7 +1114,11 @@ def hook_attrs(backward: true)
end
def project_member(user)
project_members.find_by(user_id: user)
if project_members.loaded?
project_members.find { |member| member.user_id == user.id }
else
project_members.find_by(user_id: user)
end
end
def default_branch
......
......@@ -998,7 +998,11 @@ def ci_authorized_runners
end
def notification_settings_for(source)
notification_settings.find_or_initialize_by(source: source)
if notification_settings.loaded?
notification_settings.find { |notification| notification.source == source }
else
notification_settings.find_or_initialize_by(source: source)
end
end
# Lazy load global notification setting
......
......@@ -4,28 +4,19 @@ def initialize(projects)
@projects = projects
end
def count
@projects.map do |project|
[project.id, current_count_service(project).count]
end.to_h
end
def refresh_cache
@projects.each do |project|
unless current_count_service(project).count_stored?
current_count_service(project).refresh_cache { global_count[project.id].to_i }
service = count_service.new(project)
unless service.count_stored?
service.refresh_cache { global_count[project.id].to_i }
end
end
end
def current_count_service(project)
if defined? @service
@service.project = project
else
@service = count_service.new(project)
end
def project_ids
return @projects if @projects.is_a?(ActiveRecord::Relation)
@service
@projects.map(&:id)
end
def global_count(project)
......
......@@ -2,9 +2,11 @@ module Projects
# Service class for getting and caching the number of forks of several projects
class BatchForksCountService < Projects::BatchCountService
def global_count
@global_count ||= ForkedProjectLink.where(forked_from_project: @projects.map(&:id))
.group(:forked_from_project_id)
.count
@global_count ||= begin
count_service.query(project_ids)
.group(:forked_from_project_id)
.count
end
end
def count_service
......
......@@ -2,10 +2,9 @@ module Projects
# Service class for getting and caching the number of forks of several projects
class BatchOpenIssuesCountService < Projects::BatchCountService
def global_count
@global_count ||= Issue.opened.public_only
.where(project: @projects.map(&:id))
.group(:project_id)
.count
@global_count ||= begin
count_service.query(project_ids).group(:project_id).count
end
end
def count_service
......
......@@ -7,8 +7,6 @@ class CountService < BaseCountService
# all caches.
VERSION = 1
attr_accessor :project
def initialize(project)
@project = project
end
......
......@@ -2,11 +2,15 @@ module Projects
# Service class for getting and caching the number of forks of a project.
class ForksCountService < Projects::CountService
def relation_for_count
@project.forks
self.class.query(@project.id)
end
def cache_key_name
'forks_count'
end
def self.query(project_ids)
ForkNetworkMember.where(forked_from_project: project_ids)
end
end
end
......@@ -3,13 +3,17 @@ module Projects
# project.
class OpenIssuesCountService < Projects::CountService
def relation_for_count
# We don't include confidential issues in this number since this would
# expose the number of confidential issues to non project members.
@project.issues.opened.public_only
self.class.query(@project.id)
end
def cache_key_name
'open_issues_count'
end
def self.query(project_ids)
# We don't include confidential issues in this number since this would
# expose the number of confidential issues to non project members.
Issue.opened.public_only.where(project: project_ids)
end
end
end
......@@ -96,7 +96,7 @@ class BasicProjectDetails < ProjectIdentity
expose :star_count, :forks_count
expose :created_at, :last_activity_at
def self.preload_relation(projects_relation)
def self.preload_relation(projects_relation, options = {})
projects_relation.preload(:project_feature, :route)
.preload(namespace: [:route, :owner],
tags: :taggings)
......@@ -134,18 +134,6 @@ class Project < BasicProjectDetails
expose :members do |project|
expose_url(api_v4_projects_members_path(id: project.id))
end
def self.preload_relation(projects_relation)
super(projects_relation).preload(:group)
.preload(project_group_links: :group,
fork_network: :root_project,
forked_project_link: :forked_from_project,
forked_from_project: [:route, :forks, namespace: :route, tags: :taggings])
end
def self.forks_counting_projects(projects_relation)
projects_relation + projects_relation.map(&:forked_from_project).compact
end
end
expose :archived?, as: :archived
......@@ -165,7 +153,9 @@ def self.forks_counting_projects(projects_relation)
expose :lfs_enabled?, as: :lfs_enabled
expose :creator_id
expose :namespace, using: 'API::Entities::NamespaceBasic'
expose :forked_from_project, using: Entities::BasicProjectDetails, if: lambda { |project, options| project.forked? }
expose :forked_from_project, using: Entities::BasicProjectDetails, if: lambda { |project, options| project.forked? } do |project, options|
project.fork_network_member.forked_from_project
end
expose :import_status
expose :import_error, if: lambda { |_project, options| options[:user_can_admin_project] }
......@@ -182,6 +172,20 @@ def self.forks_counting_projects(projects_relation)
expose :printing_merge_request_link_enabled
expose :statistics, using: 'API::Entities::ProjectStatistics', if: :statistics
def self.preload_relation(projects_relation, options = {})
relation = super(projects_relation).preload(:group)
.preload(project_group_links: :group,
fork_network: :root_project,
fork_network_member: [forked_from_project: [:route, namespace: :route, tags: :taggings]])
# Remove this preload once forked_project_links and forked_from_project models have been removed
relation.preload(forked_project_link: :forked_from_project)
end
def self.forks_counting_projects(projects_relation)
projects_relation + projects_relation.map(&:fork_network_member).compact.map(&:forked_from_project).compact
end
end
class ProjectStatistics < Grape::Entity
......@@ -653,16 +657,10 @@ def expose_members_count_with_descendants?(namespace, opts)
class MemberAccess < Grape::Entity
expose :access_level
expose :notification_level do |member, options|
notification = member_notification_setting(member)
::NotificationSetting.levels[notification.level] if notification
end
private
def member_notification_setting(member)
member.user.notification_settings.select do |notification|
notification.source_id == member.source_id && notification.source_type == member.source_type
end.last
# binding.pry if member.id == 5
if member.notification_setting
::NotificationSetting.levels[member.notification_setting.level]
end
end
end
......@@ -705,25 +703,36 @@ class ProjectWithAccess < Project
expose :permissions do
expose :project_access, using: Entities::ProjectAccess do |project, options|
if options.key?(:project_members)
(options[:project_members] || []).select { |member| member.source_id == project.id }.last
elsif project.project_members.any?
# This is not the bet option to search in a CollectionProxy, but if
# we use find_by we will perform another query, even if the association
# is loaded
project.project_members.select { |project_member| project_member.user_id == options[:current_user].id }.last
(options[:project_members] || []).find { |member| member.source_id == project.id }
else
project.project_member(options[:current_user])
end
end
expose :group_access, using: Entities::GroupAccess do |project, options|
if project.group
if options.key?(:group_members)
(options[:group_members] || []).select { |member| member.source_id == project.namespace_id }.last
elsif project.group.group_members.any?
project.group.group_members.select { |group_member| group_member.user_id == options[:current_user].id }.last
(options[:group_members] || []).find { |member| member.source_id == project.namespace_id }
else
project.group.group_member(options[:current_user])
end
end
end
end
def self.preload_relation(projects_relation, options = {})
relation = super(projects_relation, options)
unless options.key?(:group_members)
relation = relation.preload(group: [group_members: [:source, user: [notification_settings: :source]]])
end
unless options.key?(:project_members)
relation = relation.preload(project_members: [:source, user: [notification_settings: :source]])
end
relation
end
end
class LabelBasic < Grape::Entity
......
......@@ -82,20 +82,20 @@ def present_projects(projects, options = {})
projects = paginate(projects)
if current_user
project_members = current_user.project_members
group_members = current_user.group_members
project_members = current_user.project_members.preload(:source, user: [notification_settings: :source])
group_members = current_user.group_members.preload(:source, user: [notification_settings: :source])
end
options = options.reverse_merge(
with: current_user ? Entities::ProjectWithAccess : Entities::BasicProjectDetails,
statistics: params[:statistics],
project_members: project_members,
group_members: group_members,
project_members: nil,
group_members: nil,
current_user: current_user
)
options[:with] = Entities::BasicProjectDetails if params[:simple]
present options[:with].prepare_relation(projects), options
present options[:with].prepare_relation(projects, options), options
end
end
......
......@@ -3,13 +3,13 @@ module ProjectsRelationBuilder
extend ActiveSupport::Concern
module ClassMethods
def prepare_relation(relation)
relation = preload_relation(relation)
execute_batch_counting(relation)
relation
def prepare_relation(projects_relation, options = {})
projects_relation = preload_relation(projects_relation, options)
execute_batch_counting(projects_relation)
projects_relation
end
def preload_relation(relation)
def preload_relation(projects_relation, options = {})
raise NotImplementedError, 'self.preload_relation method must be defined and return a relation'
end
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment