Don't load unneeded elements in GroupsController#show

5 files changed, 68 insertions, 70 deletions

diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb
index 8c2053e8574..a24a0056a30 100644
--- a/app/controllers/groups_controller.rb
+++ b/app/controllers/groups_controller.rb
@@ -45,13 +45,14 @@ class GroupsController < Groups::ApplicationController
   end
 
   def show
-    setup_children(@group)
-
     respond_to do |format|
-      format.html
+      format.html do
+        @has_children = GroupDescendantsFinder.new(current_user: current_user,
+                                                   parent_group: @group,
+                                                   params: params).has_children?
+      end
 
       format.atom do
-        setup_projects
         load_events
         render layout: 'xml.atom'
       end
@@ -126,20 +127,6 @@ class GroupsController < Groups::ApplicationController
     @children = @children.page(params[:page])
   end
 
-  def setup_projects
-    set_non_archived_param
-    params[:sort] ||= 'latest_activity_desc'
-    @sort = params[:sort]
-
-    options = {}
-    options[:only_owned] = true if params[:shared] == '0'
-    options[:only_shared] = true if params[:shared] == '1'
-
-    @projects = GroupProjectsFinder.new(params: params, group: group, options: options, current_user: current_user).execute
-    @projects = @projects.includes(:namespace)
-    @projects = @projects.page(params[:page]) if params[:name].blank?
-  end
-
   def authorize_create_group!
     allowed = if params[:parent_id].present?
                 parent = Group.find_by(id: params[:parent_id])
diff --git a/app/finders/group_descendants_finder.rb b/app/finders/group_descendants_finder.rb
index 3f231bd8b10..cdd18b89525 100644
--- a/app/finders/group_descendants_finder.rb
+++ b/app/finders/group_descendants_finder.rb
@@ -19,12 +19,8 @@ class GroupDescendantsFinder
     Kaminari.paginate_array(all_required_elements, total_count: total_count)
   end
 
-  def subgroup_count
-    @subgroup_count ||= subgroups.count
-  end
-
-  def project_count
-    @project_count ||= projects.count
+  def has_children?
+    projects.any? || subgroups.any?
   end
 
   private
diff --git a/app/views/groups/show.html.haml b/app/views/groups/show.html.haml
index 75f7473d1ef..6a7d8e84636 100644
--- a/app/views/groups/show.html.haml
+++ b/app/views/groups/show.html.haml
@@ -39,7 +39,7 @@
         - else
           = link_to new_project_label, new_project_path(namespace_id: @group.id), class: "btn btn-success"
 
-  - if params[:filter].blank? && @children.empty?
+  - if params[:filter].blank? && !@has_children
     = render "shared/groups/empty_state"
   - else
     = render "children", children: @children, group: @group
diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb
index 8582f31f059..f914fd6f20a 100644
--- a/spec/controllers/groups_controller_spec.rb
+++ b/spec/controllers/groups_controller_spec.rb
@@ -150,51 +150,6 @@ describe GroupsController do
     end
   end
 
-  describe 'GET #show' do
-    context 'pagination' do
-      let(:per_page) { 3 }
-
-      before do
-        allow(Kaminari.config).to receive(:default_per_page).and_return(per_page)
-      end
-
-      context 'with only projects' do
-        let!(:other_project) { create(:project, :public, namespace: group) }
-        let!(:first_page_projects) { create_list(:project, per_page, :public, namespace: group ) }
-
-        it 'has projects on the first page' do
-          get :show, id: group.to_param, sort: 'id_desc'
-
-          expect(assigns(:children)).to contain_exactly(*first_page_projects)
-        end
-
-        it 'has projects on the second page' do
-          get :show, id: group.to_param, sort: 'id_desc', page: 2
-
-          expect(assigns(:children)).to contain_exactly(other_project)
-        end
-      end
-
-      context 'with subgroups and projects', :nested_groups do
-        let!(:first_page_subgroups) { create_list(:group,  per_page, :public,  parent: group) }
-        let!(:other_subgroup) { create(:group, :public, parent: group) }
-        let!(:next_page_projects) { create_list(:project, per_page, :public, namespace: group) }
-
-        it 'contains all subgroups' do
-          get :children, id: group.to_param, sort: 'id_asc', format: :json
-
-          expect(assigns(:children)).to contain_exactly(*first_page_subgroups)
-        end
-
-        it 'contains the project and group on the second page' do
-          get :children, id: group.to_param, sort: 'id_asc', page: 2, format: :json
-
-          expect(assigns(:children)).to contain_exactly(other_subgroup, *next_page_projects.take(per_page - 1))
-        end
-      end
-    end
-  end
-
   describe 'GET #children' do
     context 'for projects' do
       let!(:public_project) { create(:project, :public, namespace: group) }
@@ -420,6 +375,50 @@ describe GroupsController do
         end
       end
     end
+
+    context 'pagination' do
+      let(:per_page) { 3 }
+
+      before do
+        allow(Kaminari.config).to receive(:default_per_page).and_return(per_page)
+      end
+
+      context 'with only projects' do
+        let!(:other_project) { create(:project, :public, namespace: group) }
+        let!(:first_page_projects) { create_list(:project, per_page, :public, namespace: group ) }
+
+        it 'has projects on the first page' do
+          get :children, id: group.to_param, sort: 'id_desc', format: :json
+
+          expect(assigns(:children)).to contain_exactly(*first_page_projects)
+        end
+
+        it 'has projects on the second page' do
+          get :children, id: group.to_param, sort: 'id_desc', page: 2, format: :json
+
+          expect(assigns(:children)).to contain_exactly(other_project)
+        end
+      end
+
+      context 'with subgroups and projects', :nested_groups do
+        let!(:first_page_subgroups) { create_list(:group,  per_page, :public,  parent: group) }
+        let!(:other_subgroup) { create(:group, :public, parent: group) }
+        let!(:next_page_projects) { create_list(:project, per_page, :public, namespace: group) }
+
+        it 'contains all subgroups' do
+          get :children, id: group.to_param, sort: 'id_asc', format: :json
+
+          expect(assigns(:children)).to contain_exactly(*first_page_subgroups)
+        end
+
+        it 'contains the project and group on the second page' do
+          get :children, id: group.to_param, sort: 'id_asc', page: 2, format: :json
+
+          expect(assigns(:children)).to contain_exactly(other_subgroup, *next_page_projects.take(per_page - 1))
+        end
+      end
+    end
+
   end
 
   describe 'GET #issues' do
diff --git a/spec/finders/group_descendants_finder_spec.rb b/spec/finders/group_descendants_finder_spec.rb
index 1aef49613ee..4a5bdd84508 100644
--- a/spec/finders/group_descendants_finder_spec.rb
+++ b/spec/finders/group_descendants_finder_spec.rb
@@ -12,6 +12,22 @@ describe GroupDescendantsFinder do
     group.add_owner(user)
   end
 
+  describe '#has_children?' do
+    it 'is true when there are projects' do
+      create(:project, namespace: group)
+
+      expect(finder.has_children?).to be_truthy
+    end
+
+    context 'when there are subgroups', :nested_groups do
+      it 'is true when there are projects' do
+        create(:group, parent: group)
+
+        expect(finder.has_children?).to be_truthy
+      end
+    end
+  end
+
   describe '#execute' do
     it 'includes projects' do
       project = create(:project, namespace: group)