Rename `GroupHierarchy` to `GroupDescendant`

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

app/finders/group_children_finder.rb → app/finders/group_descendants_finder.rb

-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

app/models/concerns/group_hierarchy.rb → app/models/concerns/group_descendant.rb

-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

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

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

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

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

spec/finders/group_children_finder_spec.rb → spec/finders/group_descendants_finder_spec.rb

 require 'spec_helper'
-describe GroupChildrenFinder do
+describe GroupDescendantsFinder do
   let(:user) { create(:user) }
   let(:group) { create(:group) }
   let(:params) { {} }

spec/models/concerns/group_hierarchy_spec.rb → spec/models/concerns/group_descendant_spec.rb

 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