Merge branch '17673-wiki-pages-tree-structure' into 'master'

Show wiki tree structure in sidebar and pages overview

See merge request gitlab-org/gitlab!42867

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