Reduce cognitive complexity of wiki page groupingdeeply-nested-wiki-pages

Also actually test for correct grouping, instead of flat-mapping the
structure away.

2 files changed, 36 insertions, 34 deletions

diff --git a/app/models/wiki_directory.rb b/app/models/wiki_directory.rb
index 378abb94f8e..271db2b09dd 100644
--- a/app/models/wiki_directory.rb
+++ b/app/models/wiki_directory.rb
@@ -26,30 +26,35 @@ class WikiDirectory
 
   alias_method :to_param, :slug
 
-  # Sorts and groups pages by directory.
-  #
-  # pages - an array of WikiPage objects.
-  #
-  # Returns an array of WikiPage and WikiDirectory objects.
-  # The entries are sorted in the order of the input array, where
-  # directories appear in the position of their first member.
-  def self.group_by_directory(pages)
-    grouped = []
-    dirs = Hash.new do |h, k|
-      new(k).tap do |dir|
-        h[k] = dir
-        if dir.root_dir?
-          grouped << dir
-        else
-          h[dir.directory] << dir
-        end
+  class << self
+    # Sorts and groups pages by directory.
+    #
+    # pages - an array of WikiPage objects.
+    #
+    # Returns an array of WikiPage and WikiDirectory objects.
+    # The entries are sorted in the order of the input array, where
+    # directories appear in the position of their first member.
+    def group_by_directory(pages)
+      grouped = []
+      dirs = grouping_map(grouped)
+
+      (pages.presence || []).each_with_object(grouped) do |page, top_level|
+        group = page.directory.present? ? dirs[page.directory] : top_level
+
+        group << page
       end
     end
 
-    (pages.presence || []).each_with_object(grouped) do |page, top_level|
-      group = page.directory.present? ? dirs[page.directory] : top_level
+    private
 
-      group << page
+    def grouping_map(top_level)
+      Hash.new do |h, k|
+        new(k).tap do |dir|
+          h[k] = dir
+          parent = dir.root_dir? ? top_level : h[dir.directory]
+          parent << dir
+        end
+      end
     end
   end
 
diff --git a/spec/models/wiki_directory_spec.rb b/spec/models/wiki_directory_spec.rb
index 5f84204a250..5d16c75a49b 100644
--- a/spec/models/wiki_directory_spec.rb
+++ b/spec/models/wiki_directory_spec.rb
@@ -50,18 +50,18 @@ RSpec.describe WikiDirectory do
         described_class.new('dir_1/dir_1_1', [wiki.find_page('dir_1/dir_1_1/page_3')])
       end
       let(:dir_2) do
-        pages = [wiki.find_page('dir_2/page_5'),
-                 wiki.find_page('dir_2/page_4')]
+        pages = [wiki.find_page('dir_2/page_4'),
+                 wiki.find_page('dir_2/page_5')]
         described_class.new('dir_2', pages)
       end
 
       context "#list_pages" do
         shared_examples "a correct grouping" do
           let(:grouped_slugs) { grouped_entries.map(&method(:slugs)) }
-          let(:expected_slugs) { expected_grouped_entries.map(&method(:slugs)).map(&method(:match_array)) }
+          let(:expected_slugs) { expected_grouped_entries.map(&method(:slugs)) }
 
           it 'returns an array with pages and directories' do
-            expect(grouped_slugs).to match_array(expected_slugs)
+            expect(grouped_slugs).to eq expected_slugs
           end
         end
 
@@ -73,19 +73,16 @@ RSpec.describe WikiDirectory do
           it_behaves_like "a correct grouping"
         end
 
-        context 'sort by created_at' do
-          let(:grouped_entries) { described_class.group_by_directory(wiki.list_pages(sort: 'created_at')) }
-          let(:expected_grouped_entries) { [page_1, dir_1, page_dir_2, dir_2, page_6] }
-
-          it_behaves_like "a correct grouping"
-        end
-
         it 'returns an array with retained order with directories at the top' do
-          expected_order = ['dir_1/dir_1_1/page_3', 'dir_1/page_2', 'dir_2', 'dir_2/page_4', 'dir_2/page_5', 'page_1', 'page_6']
+          expected_order = [[['dir_1/dir_1_1/page_3'], 'dir_1/page_2'],
+                            'dir_2',
+                            ['dir_2/page_4', 'dir_2/page_5'],
+                            'page_1',
+                            'page_6']
 
           grouped_entries = described_class.group_by_directory(wiki.list_pages)
 
-          actual_order = grouped_entries.flat_map(&method(:slugs))
+          actual_order = grouped_entries.map(&method(:slugs))
 
           expect(actual_order).to eq(expected_order)
         end
@@ -215,6 +212,6 @@ RSpec.describe WikiDirectory do
   end
 
   def slugs(thing)
-    Array.wrap(thing.respond_to?(:pages) ? thing.pages.flat_map(&method(:slugs)) : thing.slug)
+    thing.respond_to?(:pages) ? thing.pages.map(&method(:slugs)) : thing.slug
   end
 end