Handle all cases for merging a hierarchy

The possible cases are:
 - [Array, Array]
 - [Array, Hash]
 - [Array, GroupHierarchy]
 - [Hash,Hash]
 - [Hash, GroupHierarchy]
 - [GroupHierarchy, GroupHierarchy]

1 files changed, 32 insertions, 12 deletions

diff --git a/app/models/concerns/group_hierarchy.rb b/app/models/concerns/group_hierarchy.rb
index 9999bd87686..9e02a17f4c3 100644
--- a/app/models/concerns/group_hierarchy.rb
+++ b/app/models/concerns/group_hierarchy.rb
@@ -33,7 +33,7 @@ module GroupHierarchy
     hierarchies = Array.wrap(hierarchies)
     return if hierarchies.empty?
 
-    unless hierarchies.all? { |other_base| other_base.is_a?(GroupHierarchy) }
+    unless hierarchies.all? { |hierarchy| hierarchy.is_a?(GroupHierarchy) }
       raise ArgumentError.new('element is not a hierarchy')
     end
 
@@ -50,26 +50,46 @@ module GroupHierarchy
 
   def self.merge_values(first_child, second_child)
     # When the first is an array, we need to go over every element to see if
-    # we can merge deeper.
-    if first_child.is_a?(Array)
-      first_child.map do |element|
-        if element.is_a?(Hash) && element.keys.any? { |k| second_child.keys.include?(k) }
-          element.deep_merge(second_child) { |key, first, second| merge_values(first, second) }
-        else
-          element
-        end
-      end
+    # we can merge deeper. If no match is found, we add the element to the array
+    #
+    # Handled cases:
+    # [Array, Hash]
+    if first_child.is_a?(Array) && second_child.is_a?(Hash)
+      merge_hash_into_array(first_child, second_child)
+    elsif first_child.is_a?(Hash) && second_child.is_a?(Array)
+      merge_hash_into_array(second_child, first_child)
     # If both of them are hashes, we can deep_merge with the same logic
+    #
+    # Handled cases:
+    # [Hash, Hash]
     elsif first_child.is_a?(Hash) && second_child.is_a?(Hash)
       first_child.deep_merge(second_child) { |key, first, second| merge_values(first, second) }
-    # If only one of them is a hash, we can check if the other child is already
-    # included, we don't need to do anything when it is.
+    # If only one of them is a hash, and one of them is a GroupHierachy-object
+    # we can check if its already in the hash. If so, we don't need to do anything
+    #
+    # Handled cases
+    # [Hash, GroupHierarchy]
     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
+    # combining them.
+    #
+    # Handled cases:
+    # [GroupHierarchy, Array], [GroupHierarchy, GroupHierarchy], [Array, Array]
     else
       Array.wrap(first_child) + Array.wrap(second_child)
     end
   end
+
+  def self.merge_hash_into_array(array, new_hash)
+    if mergeable_index = array.index { |element| element.is_a?(Hash) && (element.keys & new_hash.keys).any? }
+      array[mergeable_index] = merge_values(array[mergeable_index], new_hash)
+    else
+      array << new_hash
+    end
+
+    array
+  end
 end