From 2eac1537ad907f2f7e628788cf980cb7e48d3f56 Mon Sep 17 00:00:00 2001
From: Bob Van Landuyt <bob@vanlanduyt.co>
Date: Mon, 4 Sep 2017 20:01:58 +0200
Subject: [PATCH] Fetch children using new finder for the `show` of a group.

---
 app/controllers/groups_controller.rb       | 11 +++-----
 app/finders/group_children_finder.rb       | 29 +++++++++++++++-------
 app/views/groups/_children.html.haml       |  4 +++
 app/views/groups/_show_nav.html.haml       |  8 ------
 app/views/groups/show.html.haml            |  3 +--
 spec/finders/group_children_finder_spec.rb |  6 ++---
 6 files changed, 32 insertions(+), 29 deletions(-)
 create mode 100644 app/views/groups/_children.html.haml
 delete mode 100644 app/views/groups/_show_nav.html.haml

diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb
index 3769a2cde33..588995ab5a8 100644
--- a/app/controllers/groups_controller.rb
+++ b/app/controllers/groups_controller.rb
@@ -45,18 +45,15 @@ class GroupsController < Groups::ApplicationController
   end
 
   def show
-    setup_projects
+    @children = GroupChildrenFinder.new(current_user, parent_group: @group, params: params).execute
+
+    @children = @children.page(params[:page])
 
     respond_to do |format|
       format.html
 
-      format.json do
-        render json: {
-          html: view_to_html_string("dashboard/projects/_projects", locals: { projects: @projects })
-        }
-      end
-
       format.atom do
+        setup_projects
         load_events
         render layout: 'xml.atom'
       end
diff --git a/app/finders/group_children_finder.rb b/app/finders/group_children_finder.rb
index d95dfa2a877..71bef7b0ce2 100644
--- a/app/finders/group_children_finder.rb
+++ b/app/finders/group_children_finder.rb
@@ -16,27 +16,38 @@ class GroupChildrenFinder
   # This allows us to fetch only the count without loading the objects. Unless
   # the objects were already loaded.
   def total_count
-    @total_count ||= if defined?(@children)
-                       children.size
-                     else
-                       child_groups.count + projects.count
-                     end
+    @total_count ||= subgroup_count + project_count
+  end
+
+  def subgroup_count
+    @subgroup_count ||= if defined?(@children)
+                          children.count { |child| child.is_a?(Group) }
+                        else
+                          subgroups.count
+                        end
+  end
+
+  def project_count
+    @project_count ||= if defined?(@children)
+                         children.count { |child| child.is_a?(Project) }
+                       else
+                         projects.count
+                       end
   end
 
   private
 
   def children
-    @children ||= child_groups + projects
+    @children ||= subgroups + projects
   end
 
-  def child_groups
+  def subgroups
     return Group.none unless Group.supports_nested_groups?
     return Group.none unless can?(current_user, :read_group, parent_group)
 
     groups = GroupsFinder.new(current_user,
                               parent: parent_group,
-                              all_available: true,
-                              all_children_for_parent: params[:filter_groups].present?).execute
+                              all_available: true).execute
 
     groups = groups.search(params[:filter]) if params[:filter].present?
     groups = groups.includes(:route).includes(:children)
diff --git a/app/views/groups/_children.html.haml b/app/views/groups/_children.html.haml
new file mode 100644
index 00000000000..e22d9cc6013
--- /dev/null
+++ b/app/views/groups/_children.html.haml
@@ -0,0 +1,4 @@
+- if children.any?
+  render children here
+- else
+  .nothing-here-block No children found
diff --git a/app/views/groups/_show_nav.html.haml b/app/views/groups/_show_nav.html.haml
deleted file mode 100644
index 35b75bc0923..00000000000
--- a/app/views/groups/_show_nav.html.haml
+++ /dev/null
@@ -1,8 +0,0 @@
-%ul.nav-links
-  = nav_link(page: group_path(@group)) do
-    = link_to group_path(@group) do
-      Projects
-  - if Group.supports_nested_groups?
-    = nav_link(page: subgroups_group_path(@group)) do
-      = link_to subgroups_group_path(@group) do
-        Subgroups
diff --git a/app/views/groups/show.html.haml b/app/views/groups/show.html.haml
index 3ca63f9c3e0..a8842596dbd 100644
--- a/app/views/groups/show.html.haml
+++ b/app/views/groups/show.html.haml
@@ -8,7 +8,6 @@
 
 .groups-header{ class: container_class }
   .top-area
-    = render 'groups/show_nav'
     .nav-controls
       = render 'shared/projects/search_form'
       = render 'shared/projects/dropdown'
@@ -16,4 +15,4 @@
         = link_to new_project_path(namespace_id: @group.id), class: 'btn btn-new pull-right' do
           New Project
 
-  = render "projects", projects: @projects
+  = render "children", children: @children
diff --git a/spec/finders/group_children_finder_spec.rb b/spec/finders/group_children_finder_spec.rb
index afd96e27a1d..a2a24b2a12e 100644
--- a/spec/finders/group_children_finder_spec.rb
+++ b/spec/finders/group_children_finder_spec.rb
@@ -52,16 +52,16 @@ describe GroupChildrenFinder do
 
     describe '#total_count' do
       it 'counts the array children were already loaded' do
-        finder.instance_variable_set(:@children, [double])
+        finder.instance_variable_set(:@children, [build(:project)])
 
-        expect(finder).not_to receive(:child_groups)
+        expect(finder).not_to receive(:subgroups)
         expect(finder).not_to receive(:projects)
 
         expect(finder.total_count).to eq(1)
       end
 
       it 'performs a count without loading children when they are not loaded yet' do
-        expect(finder).to receive(:child_groups).and_call_original
+        expect(finder).to receive(:subgroups).and_call_original
         expect(finder).to receive(:projects).and_call_original
 
         expect(finder.total_count).to eq(2)