Commit 951abe2b authored by Bob Van Landuyt's avatar Bob Van Landuyt

Load counts everywhere we render a group tree

parent ec8a7a36
...@@ -7,7 +7,7 @@ module GroupTree ...@@ -7,7 +7,7 @@ module GroupTree
# Only show root groups if no parent-id is given # Only show root groups if no parent-id is given
@groups = groups.where(parent_id: params[:parent_id]) @groups = groups.where(parent_id: params[:parent_id])
end end
@groups = @groups.includes(:route) @groups = @groups.with_selects_for_list
.sort(@sort = params[:sort]) .sort(@sort = params[:sort])
.page(params[:page]) .page(params[:page])
......
...@@ -3,30 +3,6 @@ class GroupDescendantsFinder ...@@ -3,30 +3,6 @@ class GroupDescendantsFinder
attr_reader :current_user, :parent_group, :params attr_reader :current_user, :parent_group, :params
PROJECT_COUNT_SQL = <<~PROJECTCOUNT.freeze
(SELECT COUNT(*) AS preloaded_project_count
FROM projects
WHERE projects.namespace_id = namespaces.id
AND projects.archived IS NOT true)
PROJECTCOUNT
SUBGROUP_COUNT_SQL = <<~SUBGROUPCOUNT.freeze
(SELECT COUNT(*) AS preloaded_subgroup_count
FROM namespaces children
WHERE children.parent_id = namespaces.id)
SUBGROUPCOUNT
MEMBER_COUNT_SQL = <<~MEMBERCOUNT.freeze
(SELECT COUNT(*) AS preloaded_member_count
FROM members
WHERE members.source_type = 'Namespace'
AND members.source_id = namespaces.id
AND members.requested_at IS NULL)
MEMBERCOUNT
GROUP_SELECTS = ['namespaces.*',
PROJECT_COUNT_SQL,
SUBGROUP_COUNT_SQL,
MEMBER_COUNT_SQL].freeze
def initialize(current_user: nil, parent_group:, params: {}) def initialize(current_user: nil, parent_group:, params: {})
@current_user = current_user @current_user = current_user
@parent_group = parent_group @parent_group = parent_group
...@@ -60,7 +36,7 @@ class GroupDescendantsFinder ...@@ -60,7 +36,7 @@ class GroupDescendantsFinder
end end
def collections def collections
[subgroups.with_route.select(GROUP_SELECTS), projects] [subgroups.with_selects_for_list, projects]
end end
def paginator def paginator
...@@ -113,7 +89,7 @@ class GroupDescendantsFinder ...@@ -113,7 +89,7 @@ class GroupDescendantsFinder
projects_to_load_ancestors_of = projects.where.not(namespace: parent_group) projects_to_load_ancestors_of = projects.where.not(namespace: parent_group)
groups_to_load_ancestors_of = Group.where(id: projects_to_load_ancestors_of.select(:namespace_id)) groups_to_load_ancestors_of = Group.where(id: projects_to_load_ancestors_of.select(:namespace_id))
ancestors_for_groups(groups_to_load_ancestors_of) ancestors_for_groups(groups_to_load_ancestors_of)
.with_route.select(GROUP_SELECTS) .with_selects_for_list
end end
def subgroups def subgroups
......
module LoadedInGroupList
extend ActiveSupport::Concern
PROJECT_COUNT_SQL = <<~PROJECTCOUNT.freeze
(SELECT COUNT(*) AS preloaded_project_count
FROM projects
WHERE projects.namespace_id = namespaces.id
AND projects.archived IS NOT true)
PROJECTCOUNT
SUBGROUP_COUNT_SQL = <<~SUBGROUPCOUNT.freeze
(SELECT COUNT(*) AS preloaded_subgroup_count
FROM namespaces children
WHERE children.parent_id = namespaces.id)
SUBGROUPCOUNT
MEMBER_COUNT_SQL = <<~MEMBERCOUNT.freeze
(SELECT COUNT(*) AS preloaded_member_count
FROM members
WHERE members.source_type = 'Namespace'
AND members.source_id = namespaces.id
AND members.requested_at IS NULL)
MEMBERCOUNT
COUNT_SELECTS = ['namespaces.*',
PROJECT_COUNT_SQL,
SUBGROUP_COUNT_SQL,
MEMBER_COUNT_SQL].freeze
module ClassMethods
def with_counts
select(COUNT_SELECTS)
end
def with_selects_for_list
with_route.with_counts
end
end
def children_count
@children_count ||= project_count + subgroup_count
end
def project_count
@project_count ||= if respond_to?(:preloaded_project_count)
preloaded_project_count
else
projects.non_archived.count
end
end
def subgroup_count
@subgroup_count ||= if respond_to?(:preloaded_subgroup_count)
preloaded_subgroup_count
else
children.count
end
end
def member_count
@member_count ||= if respond_to?(:preloaded_member_count)
preloaded_member_count
else
users.count
end
end
end
...@@ -6,6 +6,7 @@ class Group < Namespace ...@@ -6,6 +6,7 @@ class Group < Namespace
include Avatarable include Avatarable
include Referable include Referable
include SelectForProjectAuthorization include SelectForProjectAuthorization
include LoadedInGroupList
include GroupDescendant include GroupDescendant
has_many :group_members, -> { where(requested_at: nil) }, dependent: :destroy, as: :source # rubocop:disable Cop/ActiveRecordDependent has_many :group_members, -> { where(requested_at: nil) }, dependent: :destroy, as: :source # rubocop:disable Cop/ActiveRecordDependent
......
...@@ -54,31 +54,6 @@ class GroupChildEntity < Grape::Entity ...@@ -54,31 +54,6 @@ class GroupChildEntity < Grape::Entity
:number_users_with_delimiter, :project_count, :subgroup_count, :can_leave, :number_users_with_delimiter, :project_count, :subgroup_count, :can_leave,
unless: lambda { |_instance, _options| project? } unless: lambda { |_instance, _options| project? }
def children_finder
@children_finder ||= GroupDescendantsFinder.new(current_user: request.current_user,
parent_group: object)
end
def children_count
@children_count ||= project_count + subgroup_count
end
def project_count
@project_count ||= if object.respond_to?(:preloaded_project_count)
object.preloaded_project_count
else
children_finder.project_count
end
end
def subgroup_count
@subgroup_count ||= if object.respond_to?(:preloaded_subgroup_count)
object.preloaded_subgroup_count
else
children_finder.subgroup_count
end
end
def leave_path def leave_path
leave_group_group_members_path(object) leave_group_group_members_path(object)
end end
...@@ -92,15 +67,10 @@ class GroupChildEntity < Grape::Entity ...@@ -92,15 +67,10 @@ class GroupChildEntity < Grape::Entity
end end
def number_projects_with_delimiter def number_projects_with_delimiter
number_with_delimiter(project_count) number_with_delimiter(object.project_count)
end end
def number_users_with_delimiter def number_users_with_delimiter
member_count = if object.respond_to?(:preloaded_member_count) number_with_delimiter(object.member_count)
object.preloaded_member_count
else
object.users.count
end
number_with_delimiter(member_count)
end end
end end
...@@ -46,18 +46,6 @@ describe GroupDescendantsFinder do ...@@ -46,18 +46,6 @@ describe GroupDescendantsFinder do
expect(finder.execute).to contain_exactly(subgroup, project) expect(finder.execute).to contain_exactly(subgroup, project)
end end
it 'includes the preloaded counts for groups' do
create(:group, parent: subgroup)
create(:project, namespace: subgroup)
subgroup.add_developer(create(:user))
found_group = finder.execute.detect { |child| child.is_a?(Group) }
expect(found_group.preloaded_project_count).to eq(1)
expect(found_group.preloaded_subgroup_count).to eq(1)
expect(found_group.preloaded_member_count).to eq(1)
end
it 'does not include subgroups the user does not have access to' do it 'does not include subgroups the user does not have access to' do
subgroup.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) subgroup.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE)
......
require 'spec_helper'
describe LoadedInGroupList do
let(:parent) { create(:group) }
subject(:found_group) { Group.with_selects_for_list.find_by(id: parent.id) }
before do
create(:group, parent: parent)
create(:project, namespace: parent)
parent.add_developer(create(:user))
end
describe '.with_selects_for_list' do
it 'includes the preloaded counts for groups' do
found_group = Group.with_selects_for_list.find_by(id: parent.id)
expect(found_group.preloaded_project_count).to eq(1)
expect(found_group.preloaded_subgroup_count).to eq(1)
expect(found_group.preloaded_member_count).to eq(1)
end
end
describe '#children_count' do
it 'counts groups and projects' do
expect(found_group.children_count).to eq(2)
end
end
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