Commit aad68a0d authored by Markus Koller's avatar Markus Koller

Correctly count wiki pages in sidebar

The existing code had two problems:

- We were only counting toplevel entries, not individual pages.
- We were showing the "View All Pages" button when the number of pages
  exactly matches the limit, even when there are no further pages.

This is now resolved by loading one page more than the limit, and using
the result count to determine if we have more pages.
parent 1d38a775
...@@ -145,7 +145,7 @@ class Projects::WikisController < Projects::ApplicationController ...@@ -145,7 +145,7 @@ class Projects::WikisController < Projects::ApplicationController
@sidebar_page = @project_wiki.find_sidebar(params[:version_id]) @sidebar_page = @project_wiki.find_sidebar(params[:version_id])
unless @sidebar_page # Fallback to default sidebar unless @sidebar_page # Fallback to default sidebar
@sidebar_wiki_entries = WikiPage.group_by_directory(@project_wiki.list_pages(limit: 15)) @sidebar_wiki_entries, @sidebar_limited = @project_wiki.sidebar_entries
end end
rescue ProjectWiki::CouldNotCreateWikiError rescue ProjectWiki::CouldNotCreateWikiError
flash[:notice] = _("Could not create Wiki Repository at this time. Please try again later.") flash[:notice] = _("Could not create Wiki Repository at this time. Please try again later.")
......
...@@ -93,6 +93,14 @@ class Wiki ...@@ -93,6 +93,14 @@ class Wiki
end end
end end
def sidebar_entries(limit: Gitlab::WikiPages::MAX_SIDEBAR_PAGES, **options)
pages = list_pages(**options.merge(limit: limit + 1))
limited = pages.size > limit
pages = pages.first(limit) if limited
[WikiPage.group_by_directory(pages), limited]
end
# Finds a page within the repository based on a tile # Finds a page within the repository based on a tile
# or slug. # or slug.
# #
......
...@@ -17,6 +17,6 @@ ...@@ -17,6 +17,6 @@
%ul.wiki-pages %ul.wiki-pages
= render @sidebar_wiki_entries, context: 'sidebar' = render @sidebar_wiki_entries, context: 'sidebar'
.block.w-100 .block.w-100
- if @sidebar_wiki_entries&.length.to_i >= 15 - if @sidebar_limited
= link_to project_wikis_pages_path(@project), class: 'btn btn-block' do = link_to project_wikis_pages_path(@project), class: 'btn btn-block' do
= s_("Wiki|View All Pages") = s_("Wiki|View All Pages")
---
title: Correctly count wiki pages in sidebar
merge_request: 30508
author:
type: fixed
...@@ -11,5 +11,8 @@ module Gitlab ...@@ -11,5 +11,8 @@ module Gitlab
# through the GitLab web interface and API: # through the GitLab web interface and API:
MAX_TITLE_BYTES = 245 # reserving 10 bytes for the file extension MAX_TITLE_BYTES = 245 # reserving 10 bytes for the file extension
MAX_DIRECTORY_BYTES = 255 MAX_DIRECTORY_BYTES = 255
# Limit the number of pages displayed in the wiki sidebar.
MAX_SIDEBAR_PAGES = 15
end end
end end
...@@ -98,13 +98,12 @@ describe Projects::WikisController do ...@@ -98,13 +98,12 @@ describe Projects::WikisController do
let(:id) { wiki_title } let(:id) { wiki_title }
it 'limits the retrieved pages for the sidebar' do it 'limits the retrieved pages for the sidebar' do
expect(controller).to receive(:load_wiki).and_return(project_wiki)
expect(project_wiki).to receive(:list_pages).with(limit: 15).and_call_original
subject subject
expect(response).to have_gitlab_http_status(:ok) expect(response).to have_gitlab_http_status(:ok)
expect(assigns(:page).title).to eq(wiki_title) expect(assigns(:page).title).to eq(wiki_title)
expect(assigns(:sidebar_wiki_entries)).to contain_exactly(an_instance_of(WikiPage))
expect(assigns(:sidebar_limited)).to be(false)
end end
context 'when page content encoding is invalid' do context 'when page content encoding is invalid' do
......
...@@ -312,6 +312,7 @@ describe "User creates wiki page" do ...@@ -312,6 +312,7 @@ describe "User creates wiki page" do
visit(project_wikis_path(project)) visit(project_wikis_path(project))
expect(page).to have_content('another') expect(page).to have_content('another')
expect(page).not_to have_link('View All Pages')
end end
context 'when there is a customized sidebar' do context 'when there is a customized sidebar' do
...@@ -328,17 +329,31 @@ describe "User creates wiki page" do ...@@ -328,17 +329,31 @@ describe "User creates wiki page" do
end end
end end
context 'when there are more than 15 existing pages' do context 'when there are 15 existing pages' do
before do before do
create(:wiki_page, wiki: wiki, title: 'home', content: 'home') (1..5).each { |i| create(:wiki_page, wiki: wiki, title: "my page #{i}") }
(1..14).each { |i| create(:wiki_page, wiki: wiki, title: "page-#{i}", content: "page #{i}") } (6..10).each { |i| create(:wiki_page, wiki: wiki, title: "parent/my page #{i}") }
(11..15).each { |i| create(:wiki_page, wiki: wiki, title: "grandparent/parent/my page #{i}") }
end end
it 'renders a default sidebar when there is no customized sidebar' do it 'shows all pages in the sidebar' do
visit(project_wikis_path(project)) visit(project_wikis_path(project))
expect(page).to have_content('View All Pages') (1..15).each { |i| expect(page).to have_content("my page #{i}") }
expect(page).to have_content('page 1') expect(page).not_to have_link('View All Pages')
end
context 'when there are more than 15 existing pages' do
before do
create(:wiki_page, wiki: wiki, title: 'my page 16')
end
it 'shows the first 15 pages in the sidebar' do
visit(project_wikis_path(project))
expect(page).to have_text('my page', count: 15)
expect(page).to have_link('View All Pages')
end
end end
end end
end end
......
...@@ -148,6 +148,44 @@ RSpec.shared_examples 'wiki model' do ...@@ -148,6 +148,44 @@ RSpec.shared_examples 'wiki model' do
end end
end end
describe '#sidebar_entries' do
before do
(1..5).each { |i| create(:wiki_page, wiki: wiki, title: "my page #{i}") }
(6..10).each { |i| create(:wiki_page, wiki: wiki, title: "parent/my page #{i}") }
(11..15).each { |i| create(:wiki_page, wiki: wiki, title: "grandparent/parent/my page #{i}") }
end
def total_pages(entries)
entries.sum do |entry|
entry.is_a?(WikiDirectory) ? entry.pages.size : 1
end
end
context 'when the number of pages does not exceed the limit' do
it 'returns all pages grouped by directory and limited is false' do
entries, limited = subject.sidebar_entries
expect(entries.size).to be(7)
expect(total_pages(entries)).to be(15)
expect(limited).to be(false)
end
end
context 'when the number of pages exceeds the limit' do
before do
create(:wiki_page, wiki: wiki, title: 'my page 16')
end
it 'returns 15 pages grouped by directory and limited is true' do
entries, limited = subject.sidebar_entries
expect(entries.size).to be(8)
expect(total_pages(entries)).to be(15)
expect(limited).to be(true)
end
end
end
describe '#find_page' do describe '#find_page' do
before do before do
subject.create_page('index page', 'This is an awesome Gollum Wiki') subject.create_page('index page', 'This is an awesome Gollum Wiki')
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment