Rename `GroupHierarchy` to `GroupDescendant`

9 files changed, 59 insertions, 34 deletions

diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb
index 1cec83a3bc6..b1deb29c61f 100644
--- a/app/controllers/groups_controller.rb
+++ b/app/controllers/groups_controller.rb
@@ -118,9 +118,9 @@ class GroupsController < Groups::ApplicationController
   protected
 
   def setup_children(parent)
-    @children = GroupChildrenFinder.new(current_user: current_user,
-                                        parent_group: parent,
-                                        params: params).execute
+    @children = GroupDescendantsFinder.new(current_user: current_user,
+                                           parent_group: parent,
+                                           params: params).execute
     @children = @children.page(params[:page])
   end
 
diff --git a/app/finders/group_children_finder.rb b/app/finders/group_descendants_finder.rb
index 7c7e7f804ec..f6b938fa732 100644
--- a/app/finders/group_children_finder.rb
+++ b/app/finders/group_descendants_finder.rb
@@ -1,4 +1,4 @@
-class GroupChildrenFinder
+class GroupDescendantsFinder
   include Gitlab::Allowable
 
   attr_reader :current_user, :parent_group, :params
@@ -38,31 +38,33 @@ class GroupChildrenFinder
   private
 
   def children
-    @children ||= subgroups.with_route.includes(:route, :parent) + projects.with_route.includes(:route, :namespace)
+    @children ||= subgroups.with_route.includes(:parent) + projects.with_route.includes(:namespace)
   end
 
-  def direct_subgroups
+  def direct_child_groups
     GroupsFinder.new(current_user,
                      parent: parent_group,
                      all_available: true).execute
   end
 
-  def all_subgroups
+  def all_descendant_groups
     Gitlab::GroupHierarchy.new(Group.where(id: parent_group)).base_and_descendants
   end
 
   def subgroups_matching_filter
-    all_subgroups.where.not(id: parent_group).search(params[:filter])
+    all_descendant_groups.where.not(id: parent_group).search(params[:filter])
   end
 
   def subgroups
     return Group.none unless Group.supports_nested_groups?
     return Group.none unless can?(current_user, :read_group, parent_group)
 
+    # When filtering subgroups, we want to find all matches withing the tree of
+    # descendants to show to the user
     groups = if params[:filter]
                subgroups_matching_filter
              else
-               direct_subgroups
+               direct_child_groups
              end
     groups.sort(params[:sort])
   end
@@ -74,7 +76,7 @@ class GroupChildrenFinder
   def projects_matching_filter
     ProjectsFinder.new(current_user: current_user).execute
       .search(params[:filter])
-      .where(namespace: all_subgroups)
+      .where(namespace: all_descendant_groups)
   end
 
   def projects
diff --git a/app/models/concerns/group_hierarchy.rb b/app/models/concerns/group_descendant.rb
index 9e02a17f4c3..89a91f87a46 100644
--- a/app/models/concerns/group_hierarchy.rb
+++ b/app/models/concerns/group_descendant.rb
@@ -1,6 +1,6 @@
-module GroupHierarchy
+module GroupDescendant
   def hierarchy(hierarchy_base = nil)
-    tree_for_child(self, self, hierarchy_base)
+    expand_hierarchy_for_child(self, self, hierarchy_base)
   end
 
   def parent
@@ -11,29 +11,29 @@ module GroupHierarchy
     end
   end
 
-  def tree_for_child(child, tree, hierarchy_base)
+  def expand_hierarchy_for_child(child, hierarchy, hierarchy_base)
     if child.parent.nil? && hierarchy_base.present?
       raise ArgumentError.new('specified base is not part of the tree')
     end
 
     if child.parent && child.parent != hierarchy_base
-      tree_for_child(child.parent,
-                     { child.parent => tree },
-                     hierarchy_base)
+      expand_hierarchy_for_child(child.parent,
+                                 { child.parent => hierarchy },
+                                 hierarchy_base)
     else
-      tree
+      hierarchy
     end
   end
 
   def merge_hierarchy(other_element, hierarchy_base = nil)
-    GroupHierarchy.merge_hierarchies([self, other_element], hierarchy_base)
+    GroupDescendant.merge_hierarchies([self, other_element], hierarchy_base)
   end
 
   def self.merge_hierarchies(hierarchies, hierarchy_base = nil)
     hierarchies = Array.wrap(hierarchies)
     return if hierarchies.empty?
 
-    unless hierarchies.all? { |hierarchy| hierarchy.is_a?(GroupHierarchy) }
+    unless hierarchies.all? { |hierarchy| hierarchy.is_a?(GroupDescendant) }
       raise ArgumentError.new('element is not a hierarchy')
     end
 
@@ -68,16 +68,16 @@ module GroupHierarchy
     # we can check if its already in the hash. If so, we don't need to do anything
     #
     # Handled cases
-    # [Hash, GroupHierarchy]
+    # [Hash, GroupDescendant]
     elsif first_child.is_a?(Hash) && first_child.keys.include?(second_child)
       first_child
     elsif second_child.is_a?(Hash) && second_child.keys.include?(first_child)
       second_child
-    # If one or both elements are a GroupHierarchy, we wrap create an array
+    # If one or both elements are a GroupDescendant, we wrap create an array
     # combining them.
     #
     # Handled cases:
-    # [GroupHierarchy, Array], [GroupHierarchy, GroupHierarchy], [Array, Array]
+    # [GroupDescendant, Array], [GroupDescendant, GroupDescendant], [Array, Array]
     else
       Array.wrap(first_child) + Array.wrap(second_child)
     end
diff --git a/app/models/group.rb b/app/models/group.rb
index 848e422e067..36439849086 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -6,7 +6,7 @@ class Group < Namespace
   include Avatarable
   include Referable
   include SelectForProjectAuthorization
-  include GroupHierarchy
+  include GroupDescendant
 
   has_many :group_members, -> { where(requested_at: nil) }, dependent: :destroy, as: :source # rubocop:disable Cop/ActiveRecordDependent
   alias_method :members, :group_members
diff --git a/app/models/project.rb b/app/models/project.rb
index c221055f8b4..bf883de48ed 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -17,7 +17,7 @@ class Project < ActiveRecord::Base
   include ProjectFeaturesCompatibility
   include SelectForProjectAuthorization
   include Routable
-  include GroupHierarchy
+  include GroupDescendant
 
   extend Gitlab::ConfigHelper
   extend Gitlab::CurrentSettings
@@ -1521,10 +1521,6 @@ class Project < ActiveRecord::Base
     map.public_path_for_source_path(path)
   end
 
-  def parent
-    namespace
-  end
-
   def parent_changed?
     namespace_id_changed?
   end
diff --git a/app/serializers/group_child_entity.rb b/app/serializers/group_child_entity.rb
index 6c795da8f88..8a2624c3d32 100644
--- a/app/serializers/group_child_entity.rb
+++ b/app/serializers/group_child_entity.rb
@@ -55,8 +55,8 @@ class GroupChildEntity < Grape::Entity
          unless: lambda { |_instance, _options| project? }
 
   def children_finder
-    @children_finder ||= GroupChildrenFinder.new(current_user: request.current_user,
-                                                 parent_group: object)
+    @children_finder ||= GroupDescendantsFinder.new(current_user: request.current_user,
+                                                    parent_group: object)
   end
 
   def children_count
diff --git a/app/serializers/group_child_serializer.rb b/app/serializers/group_child_serializer.rb
index fb47bf0b4eb..1b6b2ad6e08 100644
--- a/app/serializers/group_child_serializer.rb
+++ b/app/serializers/group_child_serializer.rb
@@ -24,10 +24,10 @@ class GroupChildSerializer < BaseSerializer
   protected
 
   def represent_hierarchies(children, opts)
-    if children.is_a?(GroupHierarchy)
+    if children.is_a?(GroupDescendant)
       represent_hierarchy(children.hierarchy(hierarchy_root), opts).first
     else
-      hierarchies = Array.wrap(GroupHierarchy.merge_hierarchies(children, hierarchy_root))
+      hierarchies = Array.wrap(GroupDescendant.merge_hierarchies(children, hierarchy_root))
       hierarchies.map { |hierarchy| represent_hierarchy(hierarchy, opts) }.flatten
     end
   end
diff --git a/spec/finders/group_children_finder_spec.rb b/spec/finders/group_descendants_finder_spec.rb
index 8257e158b06..c1268a486cf 100644
--- a/spec/finders/group_children_finder_spec.rb
+++ b/spec/finders/group_descendants_finder_spec.rb
@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-describe GroupChildrenFinder do
+describe GroupDescendantsFinder do
   let(:user) { create(:user) }
   let(:group) { create(:group) }
   let(:params) { {} }
diff --git a/spec/models/concerns/group_hierarchy_spec.rb b/spec/models/concerns/group_descendant_spec.rb
index fe30895f15e..87eee515cde 100644
--- a/spec/models/concerns/group_hierarchy_spec.rb
+++ b/spec/models/concerns/group_descendant_spec.rb
@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-describe GroupHierarchy, :nested_groups do
+describe GroupDescendant, :nested_groups do
   let(:parent) { create(:group) }
   let(:subgroup) { create(:group, parent: parent) }
   let(:subsub_group) { create(:group, parent: subgroup) }
@@ -141,6 +141,33 @@ describe GroupHierarchy, :nested_groups do
 
         expect(described_class.merge_hierarchies([parent, subgroup])).to eq(expected_hierarchy)
       end
+
+      it 'merges complex hierarchies' do
+        project = create(:project, namespace: parent)
+        sub_project = create(:project, namespace: subgroup)
+        subsubsub_group = create(:group, parent: subsub_group)
+        subsub_project = create(:project, namespace: subsub_group)
+        subsubsub_project = create(:project, namespace: subsubsub_group)
+        other_subgroup = create(:group, parent: parent)
+        other_subproject = create(:project, namespace: other_subgroup)
+
+        projects = [project, subsubsub_project, sub_project, other_subproject, subsub_project]
+
+        expected_hierarchy = [
+          project,
+          {
+            subgroup => [
+              { subsub_group => [{ subsubsub_group => subsubsub_project }, subsub_project] },
+              sub_project
+            ]
+          },
+          { other_subgroup => other_subproject }
+        ]
+
+        actual_hierarchy = described_class.merge_hierarchies(projects, parent)
+
+        expect(actual_hierarchy).to eq(expected_hierarchy)
+      end
     end
   end
 end