Show wiki tree structure in sidebar and pages overview

Replaces `WikiPage.group_by_directory` with `WikiDirectory.group_pages`,
which groups the pages recursively by their path.

The views were already set up for recursive partials, so we only had
to change them to show the basename of each directory, rather than
their full path. This is done through a new `WikiDirectory#title`
helper.

app/controllers/concerns/wiki_actions.rb

@@ -44,7 +44,7 @@ module WikiActions
       wiki.list_pages(sort: params[:sort], direction: params[:direction])
     ).page(params[:page])
 
-    @wiki_entries = WikiPage.group_by_directory(@wiki_pages)
+    @wiki_entries = WikiDirectory.group_pages(@wiki_pages)
 
     render 'shared/wikis/pages'
   end

app/models/wiki.rb

@@ -103,7 +103,7 @@ class Wiki
     limited = pages.size > limit
     pages = pages.first(limit) if limited
 
-    [WikiPage.group_by_directory(pages), limited]
+    [WikiDirectory.group_pages(pages), limited]
   end
 
   # Finds a page within the repository based on a tile

app/models/wiki_directory.rb

@@ -3,13 +3,46 @@
 class WikiDirectory
   include ActiveModel::Validations
 
-  attr_accessor :slug, :pages
+  attr_accessor :slug, :entries
 
   validates :slug, presence: true
 
-  def initialize(slug, pages = [])
+  # Groups a list of wiki pages into a nested collection of WikiPage and WikiDirectory objects,
+  # preserving the order of the passed pages.
+  #
+  # Returns an array with all entries for the toplevel directory.
+  #
+  # @param [Array<WikiPage>] pages
+  # @return [Array<WikiPage, WikiDirectory>]
+  #
+  def self.group_pages(pages)
+    # Build a hash to map paths to created WikiDirectory objects,
+    # and recursively create them for each level of the path.
+    # For the toplevel directory we use '' as path, as that's what WikiPage#directory returns.
+    directories = Hash.new do |_, path|
+      directories[path] = new(path).tap do |directory|
+        if path.present?
+          parent = File.dirname(path)
+          parent = '' if parent == '.'
+          directories[parent].entries << directory
+        end
+      end
+    end
+
+    pages.each do |page|
+      directories[page.directory].entries << page
+    end
+
+    directories[''].entries
+  end
+
+  def initialize(slug, entries = [])
     @slug = slug
-    @pages = pages
+    @entries = entries
+  end
+
+  def title
+    WikiPage.unhyphenize(File.basename(slug))
   end
 
   # Relative path to the partial to be used when rendering collections

app/models/wiki_page.rb

@@ -31,29 +31,6 @@ class WikiPage
 
   alias_method :==, :eql?
 
-  # Sorts and groups pages by directory.
-  #
-  # pages - an array of WikiPage objects.
-  #
-  # Returns an array of WikiPage and WikiDirectory objects. The entries are
-  # sorted by alphabetical order (directories and pages inside each directory).
-  # Pages at the root level come before everything.
-  def self.group_by_directory(pages)
-    return [] if pages.blank?
-
-    pages.each_with_object([]) do |page, grouped_pages|
-      next grouped_pages << page unless page.directory.present?
-
-      directory = grouped_pages.find do |obj|
-        obj.is_a?(WikiDirectory) && obj.slug == page.directory
-      end
-
-      next directory.pages << page if directory
-
-      grouped_pages << WikiDirectory.new(page.directory, [page])
-    end
-  end
-
   def self.unhyphenize(name)
     name.gsub(/-+/, ' ')
   end

app/views/shared/wikis/_pages_wiki_page.html.haml

 %li
-  = link_to wiki_page.title, wiki_page_path(@wiki, wiki_page), data: { qa_selector: 'wiki_page_link', qa_page_name: wiki_page.slug }
+  = link_to wiki_page.human_title, wiki_page_path(@wiki, wiki_page), data: { qa_selector: 'wiki_page_link', qa_page_name: wiki_page.slug }
   %small (#{wiki_page.format})
   .float-right
     - if wiki_page.last_version

app/views/shared/wikis/_wiki_directory.html.haml

 %li{ data: { qa_selector: 'wiki_directory_content' } }
-  = wiki_directory.slug
+  = wiki_directory.title
   %ul
-    = render wiki_directory.pages, context: context
+    = render wiki_directory.entries, context: context

changelogs/unreleased/17673-wiki-pages-tree-structure.yml

+---
+title: Show wiki tree structure in sidebar and pages overview
+merge_request: 42867
+author:
+type: changed

ee/spec/factories/group_wikis.rb

@@ -2,6 +2,6 @@
 
 FactoryBot.define do
   factory :group_wiki, parent: :wiki do
-    container { association(:group, :wiki_repo) }
+    container { association(:group) }
   end
 end

qa/qa/specs/features/browser_ui/3_create/wiki/project_based_directory_management_spec.rb

@@ -4,7 +4,7 @@ module QA
   RSpec.describe 'Create' do
     context 'Wiki' do
       let(:initial_wiki) { Resource::Wiki::ProjectPage.fabricate_via_api! }
-      let(:new_path) { "a/new/path" }
+      let(:new_path) { "a/new/path-with-spaces" }
 
       before do
         Flow::Login.sign_in
@@ -23,7 +23,9 @@ module QA
         Page::Project::Wiki::Edit.perform(&:click_save_changes)
 
         Page::Project::Wiki::Show.perform do |wiki|
-          expect(wiki).to have_directory(new_path)
+          expect(wiki).to have_directory('a')
+          expect(wiki).to have_directory('new')
+          expect(wiki).to have_directory('path with spaces')
         end
       end
     end

spec/factories/wiki_pages.rb

@@ -9,7 +9,7 @@ FactoryBot.define do
       content { 'Content for wiki page' }
       format { :markdown }
       message { nil }
-      project { association(:project, :wiki_repo) }
+      project { association(:project) }
       container { project }
       wiki { association(:wiki, container: container) }
       page { OpenStruct.new(url_path: title) }
@@ -18,6 +18,7 @@ FactoryBot.define do
     initialize_with do
       new(wiki, page).tap do |page|
         page.attributes = {
+          slug: title&.tr(' ', '-'),
           title: title,
           content: content,
           format: format

spec/factories/wikis.rb

@@ -3,7 +3,7 @@
 FactoryBot.define do
   factory :wiki do
     transient do
-      container { association(:project, :wiki_repo) }
+      container { association(:project) }
       user { association(:user) }
     end
 
@@ -12,7 +12,7 @@ FactoryBot.define do
 
     factory :project_wiki do
       transient do
-        project { association(:project, :wiki_repo) }
+        project { association(:project) }
       end
 
       container { project }

spec/models/wiki_directory_spec.rb

@@ -3,43 +3,97 @@
 require 'spec_helper'
 
 RSpec.describe WikiDirectory do
-  describe 'validations' do
-    subject { build(:wiki_directory) }
+  subject(:directory) { build(:wiki_directory) }
 
+  describe 'validations' do
     it { is_expected.to validate_presence_of(:slug) }
   end
 
+  describe '.group_pages' do
+    let_it_be(:toplevel1) { build(:wiki_page, title: 'aaa-toplevel1') }
+    let_it_be(:toplevel2) { build(:wiki_page, title: 'zzz-toplevel2') }
+    let_it_be(:toplevel3) { build(:wiki_page, title: 'zzz-toplevel3') }
+    let_it_be(:child1) { build(:wiki_page, title: 'parent1/child1') }
+    let_it_be(:child2) { build(:wiki_page, title: 'parent1/child2') }
+    let_it_be(:child3) { build(:wiki_page, title: 'parent2/child3') }
+    let_it_be(:grandchild1) { build(:wiki_page, title: 'parent1/subparent/grandchild1') }
+    let_it_be(:grandchild2) { build(:wiki_page, title: 'parent1/subparent/grandchild2') }
+
+    it 'returns a nested array of entries' do
+      entries = described_class.group_pages(
+        [toplevel1, toplevel2, toplevel3, child1, child2, child3, grandchild1, grandchild2].sort_by(&:title)
+      )
+
+      expect(entries).to match([
+        toplevel1,
+        a_kind_of(WikiDirectory).and(
+          having_attributes(
+            slug: 'parent1', entries: [
+              child1,
+              child2,
+              a_kind_of(WikiDirectory).and(
+                having_attributes(
+                  slug: 'parent1/subparent',
+                  entries: [grandchild1, grandchild2]
+                )
+              )
+            ]
+          )
+        ),
+        a_kind_of(WikiDirectory).and(
+          having_attributes(
+            slug: 'parent2',
+            entries: [child3]
+          )
+        ),
+        toplevel2,
+        toplevel3
+      ])
+    end
+  end
+
   describe '#initialize' do
-    context 'when there are pages' do
-      let(:pages) { [build(:wiki_page)] }
-      let(:directory) { described_class.new('/path_up_to/dir', pages) }
+    context 'when there are entries' do
+      let(:entries) { [build(:wiki_page)] }
+      let(:directory) { described_class.new('/path_up_to/dir', entries) }
 
       it 'sets the slug attribute' do
         expect(directory.slug).to eq('/path_up_to/dir')
       end
 
-      it 'sets the pages attribute' do
-        expect(directory.pages).to eq(pages)
+      it 'sets the entries attribute' do
+        expect(directory.entries).to eq(entries)
       end
     end
 
-    context 'when there are no pages' do
+    context 'when there are no entries' do
       let(:directory) { described_class.new('/path_up_to/dir') }
 
       it 'sets the slug attribute' do
         expect(directory.slug).to eq('/path_up_to/dir')
       end
 
-      it 'sets the pages attribute to an empty array' do
-        expect(directory.pages).to eq([])
+      it 'sets the entries attribute to an empty array' do
+        expect(directory.entries).to eq([])
       end
     end
   end
 
+  describe '#title' do
+    it 'returns the basename of the directory, with hyphens replaced by spaces' do
+      directory.slug = 'parent'
+      expect(directory.title).to eq('parent')
+
+      directory.slug = 'parent/child'
+      expect(directory.title).to eq('child')
+
+      directory.slug = 'parent/child-foo'
+      expect(directory.title).to eq('child foo')
+    end
+  end
+
   describe '#to_partial_path' do
     it 'returns the relative path to the partial to be used' do
-      directory = build(:wiki_directory)
-
       expect(directory.to_partial_path).to eq('../shared/wikis/wiki_directory')
     end
   end

spec/models/wiki_page_spec.rb

@@ -23,89 +23,6 @@ RSpec.describe WikiPage do
     stub_feature_flags(Gitlab::WikiPages::FrontMatterParser::FEATURE_FLAG => thing)
   end
 
-  describe '.group_by_directory' do
-    context 'when there are no pages' do
-      it 'returns an empty array' do
-        expect(described_class.group_by_directory(nil)).to eq([])
-        expect(described_class.group_by_directory([])).to eq([])
-      end
-    end
-
-    context 'when there are pages' do
-      before do
-        wiki.create_page('dir_1/dir_1_1/page_3', 'content')
-        wiki.create_page('page_1', 'content')
-        wiki.create_page('dir_1/page_2', 'content')
-        wiki.create_page('dir_2', 'page with dir name')
-        wiki.create_page('dir_2/page_5', 'content')
-        wiki.create_page('page_6', 'content')
-        wiki.create_page('dir_2/page_4', 'content')
-      end
-
-      let(:page_1) { wiki.find_page('page_1') }
-      let(:page_6) { wiki.find_page('page_6') }
-      let(:page_dir_2) { wiki.find_page('dir_2') }
-
-      let(:dir_1) do
-        WikiDirectory.new('dir_1', [wiki.find_page('dir_1/page_2')])
-      end
-
-      let(:dir_1_1) do
-        WikiDirectory.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')]
-        WikiDirectory.new('dir_2', pages)
-      end
-
-      describe "#list_pages" do
-        context 'sort by title' do
-          let(:grouped_entries) { described_class.group_by_directory(wiki.list_pages) }
-          let(:expected_grouped_entries) { [dir_1_1, dir_1, page_dir_2, dir_2, page_1, page_6] }
-
-          it 'returns an array with pages and directories' do
-            grouped_entries.each_with_index do |page_or_dir, i|
-              expected_page_or_dir = expected_grouped_entries[i]
-              expected_slugs = get_slugs(expected_page_or_dir)
-              slugs = get_slugs(page_or_dir)
-
-              expect(slugs).to match_array(expected_slugs)
-            end
-          end
-        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) { [dir_1_1, page_1, dir_1, page_dir_2, dir_2, page_6] }
-
-          it 'returns an array with pages and directories' do
-            grouped_entries.each_with_index do |page_or_dir, i|
-              expected_page_or_dir = expected_grouped_entries[i]
-              expected_slugs = get_slugs(expected_page_or_dir)
-              slugs = get_slugs(page_or_dir)
-
-              expect(slugs).to match_array(expected_slugs)
-            end
-          end
-        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']
-
-          grouped_entries = described_class.group_by_directory(wiki.list_pages)
-
-          actual_order =
-            grouped_entries.flat_map do |page_or_dir|
-              get_slugs(page_or_dir)
-            end
-          expect(actual_order).to eq(expected_order)
-        end
-      end
-    end
-  end
-
   describe '#front_matter' do
     let_it_be(:project) { create(:project) }
     let(:container) { project }
@@ -993,14 +910,4 @@ RSpec.describe WikiPage do
       )
     end
   end
-
-  private
-
-  def get_slugs(page_or_dir)
-    if page_or_dir.is_a? WikiPage
-      [page_or_dir.slug]
-    else
-      page_or_dir.pages.present? ? page_or_dir.pages.map(&:slug) : []
-    end
-  end
 end

spec/support/shared_examples/models/wiki_shared_examples.rb

@@ -164,7 +164,7 @@ RSpec.shared_examples 'wiki model' do
 
     def total_pages(entries)
       entries.sum do |entry|
-        entry.is_a?(WikiDirectory) ? entry.pages.size : 1
+        entry.is_a?(WikiDirectory) ? total_pages(entry.entries) : 1
       end
     end