From 866b58a54e95415ad74b8f004db40abb4343c753 Mon Sep 17 00:00:00 2001 From: Igor Date: Thu, 4 Apr 2019 16:28:56 +0000 Subject: [PATCH] Allow to sort wiki pages by date and title - Add controls for sorting by title and date - Execute Gitaly call which now accepts sorting params for wikis --- app/controllers/projects/wikis_controller.rb | 5 +- app/helpers/wiki_helper.rb | 20 +++++ app/models/project_wiki.rb | 16 +++- app/models/wiki_page.rb | 19 ++-- app/views/projects/wikis/pages.html.haml | 13 +++ .../unreleased/id-51433-sort-wiki-by-date.yml | 5 ++ lib/gitlab/git/wiki.rb | 10 ++- lib/gitlab/gitaly_client/wiki_service.rb | 9 +- locale/gitlab.pot | 9 ++ .../wiki/user_views_wiki_pages_spec.rb | 89 +++++++++++++++++++ spec/helpers/wiki_helper_spec.rb | 52 +++++++++++ spec/models/wiki_page_spec.rb | 47 ++++++---- 12 files changed, 260 insertions(+), 34 deletions(-) create mode 100644 changelogs/unreleased/id-51433-sort-wiki-by-date.yml create mode 100644 spec/features/projects/wiki/user_views_wiki_pages_spec.rb diff --git a/app/controllers/projects/wikis_controller.rb b/app/controllers/projects/wikis_controller.rb index da2420633ef..88910c91763 100644 --- a/app/controllers/projects/wikis_controller.rb +++ b/app/controllers/projects/wikis_controller.rb @@ -16,7 +16,10 @@ class Projects::WikisController < Projects::ApplicationController end def pages - @wiki_pages = Kaminari.paginate_array(@project_wiki.pages).page(params[:page]) + @wiki_pages = Kaminari.paginate_array( + @project_wiki.pages(sort: params[:sort], direction: params[:direction]) + ).page(params[:page]) + @wiki_entries = WikiPage.group_by_directory(@wiki_pages) end diff --git a/app/helpers/wiki_helper.rb b/app/helpers/wiki_helper.rb index 647f34e57ed..edd48f82729 100644 --- a/app/helpers/wiki_helper.rb +++ b/app/helpers/wiki_helper.rb @@ -47,4 +47,24 @@ module WikiHelper def wiki_attachment_upload_url expose_url(api_v4_projects_wikis_attachments_path(id: @project.id)) end + + def wiki_sort_controls(project, sort, direction) + sort ||= ProjectWiki::TITLE_ORDER + link_class = 'btn btn-default has-tooltip reverse-sort-btn qa-reverse-sort' + reversed_direction = direction == 'desc' ? 'asc' : 'desc' + icon_class = direction == 'desc' ? 'highest' : 'lowest' + + link_to(project_wikis_pages_path(project, sort: sort, direction: reversed_direction), + type: 'button', class: link_class, title: _('Sort direction')) do + sprite_icon("sort-#{icon_class}", size: 16) + end + end + + def wiki_sort_title(key) + if key == ProjectWiki::CREATED_AT_ORDER + s_("Wiki|Created date") + else + s_("Wiki|Title") + end + end end diff --git a/app/models/project_wiki.rb b/app/models/project_wiki.rb index 268706a6aea..23ddd708396 100644 --- a/app/models/project_wiki.rb +++ b/app/models/project_wiki.rb @@ -13,6 +13,11 @@ class ProjectWiki CouldNotCreateWikiError = Class.new(StandardError) SIDEBAR = '_sidebar' + TITLE_ORDER = 'title' + CREATED_AT_ORDER = 'created_at' + DIRECTION_DESC = 'desc' + DIRECTION_ASC = 'asc' + # Returns a string describing what went wrong after # an operation fails. attr_reader :error_message @@ -82,8 +87,15 @@ class ProjectWiki # Returns an Array of GitLab WikiPage instances or an # empty Array if this Wiki has no pages. - def pages(limit: 0) - wiki.pages(limit: limit).map { |page| WikiPage.new(self, page, true) } + def pages(limit: 0, sort: nil, direction: DIRECTION_ASC) + sort ||= TITLE_ORDER + direction_desc = direction == DIRECTION_DESC + + wiki.pages( + limit: limit, sort: sort, direction_desc: direction_desc + ).map do |page| + WikiPage.new(self, page, true) + end end # Finds a page within the repository based on a tile diff --git a/app/models/wiki_page.rb b/app/models/wiki_page.rb index 64daba81dcf..909da4316d0 100644 --- a/app/models/wiki_page.rb +++ b/app/models/wiki_page.rb @@ -28,16 +28,15 @@ class WikiPage def self.group_by_directory(pages) return [] if pages.blank? - pages.sort_by { |page| [page.directory, page.slug] } - .group_by(&:directory) - .map do |dir, pages| - if dir.present? - WikiDirectory.new(dir, pages) - else - pages - end - end - .flatten + pages.each_with_object([]) do |page, grouped_pages| + next grouped_pages << page unless page.directory.present? + + directory = grouped_pages.find { |dir| dir.slug == page.directory } + + next directory.pages << page if directory + + grouped_pages << WikiDirectory.new(page.directory, [page]) + end end def self.unhyphenize(name) diff --git a/app/views/projects/wikis/pages.html.haml b/app/views/projects/wikis/pages.html.haml index 94267b6e0cf..77fdf7f001c 100644 --- a/app/views/projects/wikis/pages.html.haml +++ b/app/views/projects/wikis/pages.html.haml @@ -2,6 +2,7 @@ - add_to_breadcrumbs "Wiki", project_wiki_path(@project, :home) - breadcrumb_title s_("Wiki|Pages") - page_title s_("Wiki|Pages"), _("Wiki") +- sort_title = wiki_sort_title(params[:sort]) %div{ class: container_class } .wiki-page-header @@ -15,6 +16,18 @@ = icon('cloud-download') = _("Clone repository") + .dropdown.inline.wiki-sort-dropdown + .btn-group{ role: 'group' } + .btn-group{ role: 'group' } + %button.dropdown-toggle{ type: 'button', data: { toggle: 'dropdown', display: 'static' }, class: 'btn btn-default' } + = sort_title + = icon('chevron-down') + %ul.dropdown-menu.dropdown-menu-right.dropdown-menu-selectable.dropdown-menu-sort + %li + = sortable_item(s_("Wiki|Title"), project_wikis_pages_path(@project, sort: ProjectWiki::TITLE_ORDER), sort_title) + = sortable_item(s_("Wiki|Created date"), project_wikis_pages_path(@project, sort: ProjectWiki::CREATED_AT_ORDER), sort_title) + = wiki_sort_controls(@project, params[:sort], params[:direction]) + %ul.wiki-pages-list.content-list = render @wiki_entries, context: 'pages' diff --git a/changelogs/unreleased/id-51433-sort-wiki-by-date.yml b/changelogs/unreleased/id-51433-sort-wiki-by-date.yml new file mode 100644 index 00000000000..86fcf195fa7 --- /dev/null +++ b/changelogs/unreleased/id-51433-sort-wiki-by-date.yml @@ -0,0 +1,5 @@ +--- +title: Allow to sort wiki pages by date and title +merge_request: 25365 +author: +type: added diff --git a/lib/gitlab/git/wiki.rb b/lib/gitlab/git/wiki.rb index c43331bed60..a0dd4a24363 100644 --- a/lib/gitlab/git/wiki.rb +++ b/lib/gitlab/git/wiki.rb @@ -86,9 +86,9 @@ module Gitlab end end - def pages(limit: 0) + def pages(limit: 0, sort: nil, direction_desc: false) wrapped_gitaly_errors do - gitaly_get_all_pages(limit: limit) + gitaly_get_all_pages(limit: limit, sort: sort, direction_desc: direction_desc) end end @@ -168,8 +168,10 @@ module Gitlab Gitlab::Git::WikiFile.new(wiki_file) end - def gitaly_get_all_pages(limit: 0) - gitaly_wiki_client.get_all_pages(limit: limit).map do |wiki_page, version| + def gitaly_get_all_pages(limit: 0, sort: nil, direction_desc: false) + gitaly_wiki_client.get_all_pages( + limit: limit, sort: sort, direction_desc: direction_desc + ).map do |wiki_page, version| Gitlab::Git::WikiPage.new(wiki_page, version) end end diff --git a/lib/gitlab/gitaly_client/wiki_service.rb b/lib/gitlab/gitaly_client/wiki_service.rb index 15c9463e2f2..e036cdcd800 100644 --- a/lib/gitlab/gitaly_client/wiki_service.rb +++ b/lib/gitlab/gitaly_client/wiki_service.rb @@ -87,8 +87,13 @@ module Gitlab wiki_page_from_iterator(response) end - def get_all_pages(limit: 0) - request = Gitaly::WikiGetAllPagesRequest.new(repository: @gitaly_repo, limit: limit) + def get_all_pages(limit: 0, sort: nil, direction_desc: false) + sort_value = Gitaly::WikiGetAllPagesRequest::SortBy.resolve(sort.to_s.upcase.to_sym) + + params = { repository: @gitaly_repo, limit: limit, direction_desc: direction_desc } + params[:sort] = sort_value if sort_value + + request = Gitaly::WikiGetAllPagesRequest.new(params) response = GitalyClient.call(@repository.storage, :wiki_service, :wiki_get_all_pages, request, timeout: GitalyClient.medium_timeout) pages = [] diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 9de34dd92ea..669028625ab 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -7417,6 +7417,9 @@ msgstr "" msgid "Sort by" msgstr "" +msgid "Sort direction" +msgstr "" + msgid "SortOptions|Access level, ascending" msgstr "" @@ -9197,6 +9200,9 @@ msgstr "" msgid "Wiki|Create page" msgstr "" +msgid "Wiki|Created date" +msgstr "" + msgid "Wiki|Edit Page" msgstr "" @@ -9215,6 +9221,9 @@ msgstr "" msgid "Wiki|Pages" msgstr "" +msgid "Wiki|Title" +msgstr "" + msgid "Wiki|Wiki Pages" msgstr "" diff --git a/spec/features/projects/wiki/user_views_wiki_pages_spec.rb b/spec/features/projects/wiki/user_views_wiki_pages_spec.rb new file mode 100644 index 00000000000..5c16d7783f0 --- /dev/null +++ b/spec/features/projects/wiki/user_views_wiki_pages_spec.rb @@ -0,0 +1,89 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'User views wiki pages' do + include WikiHelpers + + let(:user) { create(:user) } + let(:project) { create(:project, :wiki_repo, namespace: user.namespace) } + + let!(:wiki_page1) do + create(:wiki_page, wiki: project.wiki, attrs: { title: '3 home', content: '3' }) + end + let!(:wiki_page2) do + create(:wiki_page, wiki: project.wiki, attrs: { title: '1 home', content: '1' }) + end + let!(:wiki_page3) do + create(:wiki_page, wiki: project.wiki, attrs: { title: '2 home', content: '2' }) + end + + let(:pages) do + page.find('.wiki-pages-list').all('li').map { |li| li.find('a') } + end + + before do + project.add_maintainer(user) + sign_in(user) + visit(project_wikis_pages_path(project)) + end + + context 'ordered by title' do + let(:pages_ordered_by_title) { [wiki_page2, wiki_page3, wiki_page1] } + + context 'asc' do + it 'pages are displayed in direct order' do + pages.each.with_index do |page_title, index| + expect(page_title.text).to eq(pages_ordered_by_title[index].title) + end + end + end + + context 'desc' do + before do + page.within('.wiki-sort-dropdown') do + page.find('.qa-reverse-sort').click + end + end + + it 'pages are displayed in reversed order' do + pages.reverse_each.with_index do |page_title, index| + expect(page_title.text).to eq(pages_ordered_by_title[index].title) + end + end + end + end + + context 'ordered by created_at' do + let(:pages_ordered_by_created_at) { [wiki_page1, wiki_page2, wiki_page3] } + + before do + page.within('.wiki-sort-dropdown') do + click_button('Title') + click_link('Created date') + end + end + + context 'asc' do + it 'pages are displayed in direct order' do + pages.each.with_index do |page_title, index| + expect(page_title.text).to eq(pages_ordered_by_created_at[index].title) + end + end + end + + context 'desc' do + before do + page.within('.wiki-sort-dropdown') do + page.find('.qa-reverse-sort').click + end + end + + it 'pages are displayed in reversed order' do + pages.reverse_each.with_index do |page_title, index| + expect(page_title.text).to eq(pages_ordered_by_created_at[index].title) + end + end + end + end +end diff --git a/spec/helpers/wiki_helper_spec.rb b/spec/helpers/wiki_helper_spec.rb index 92c6f27a867..8eab40aeaf3 100644 --- a/spec/helpers/wiki_helper_spec.rb +++ b/spec/helpers/wiki_helper_spec.rb @@ -18,4 +18,56 @@ describe WikiHelper do end end end + + describe '#wiki_sort_controls' do + let(:project) { create(:project) } + let(:wiki_link) { helper.wiki_sort_controls(project, sort, direction) } + let(:classes) { "btn btn-default has-tooltip reverse-sort-btn qa-reverse-sort" } + + def expected_link(sort, direction, icon_class) + path = "/#{project.full_path}/wikis/pages?direction=#{direction}&sort=#{sort}" + + helper.link_to(path, type: 'button', class: classes, title: 'Sort direction') do + helper.sprite_icon("sort-#{icon_class}", size: 16) + end + end + + context 'initial call' do + let(:sort) { nil } + let(:direction) { nil } + + it 'renders with default values' do + expect(wiki_link).to eq(expected_link('title', 'desc', 'lowest')) + end + end + + context 'sort by title' do + let(:sort) { 'title' } + let(:direction) { 'asc' } + + it 'renders a link with opposite direction' do + expect(wiki_link).to eq(expected_link('title', 'desc', 'lowest')) + end + end + + context 'sort by created_at' do + let(:sort) { 'created_at' } + let(:direction) { 'desc' } + + it 'renders a link with opposite direction' do + expect(wiki_link).to eq(expected_link('created_at', 'asc', 'highest')) + end + end + end + + describe '#wiki_sort_title' do + it 'returns a title corresponding to a key' do + expect(helper.wiki_sort_title('created_at')).to eq('Created date') + expect(helper.wiki_sort_title('title')).to eq('Title') + end + + it 'defaults to Title if a key is unknown' do + expect(helper.wiki_sort_title('unknown')).to eq('Title') + end + end end diff --git a/spec/models/wiki_page_spec.rb b/spec/models/wiki_page_spec.rb index e68da67818a..cacdb0e0595 100644 --- a/spec/models/wiki_page_spec.rb +++ b/spec/models/wiki_page_spec.rb @@ -20,12 +20,16 @@ describe WikiPage do context 'when there are pages' do before do create_page('dir_1/dir_1_1/page_3', 'content') + create_page('page_1', 'content') create_page('dir_1/page_2', 'content') create_page('dir_2/page_5', 'content') + create_page('page_6', 'content') create_page('dir_2/page_4', 'content') - create_page('page_1', 'content') end + let(:page_1) { wiki.find_page('page_1') } + let(:page_6) { wiki.find_page('page_6') } + let(:dir_1) do WikiDirectory.new('dir_1', [wiki.find_page('dir_1/page_2')]) end @@ -38,25 +42,38 @@ describe WikiPage do WikiDirectory.new('dir_2', pages) end - it 'returns an array with pages and directories' do - expected_grouped_entries = [page_1, dir_1, dir_1_1, dir_2] + context 'sort by title' do + let(:grouped_entries) { described_class.group_by_directory(wiki.pages) } + let(:expected_grouped_entries) { [dir_1_1, dir_1, dir_2, page_1, page_6] } - grouped_entries = described_class.group_by_directory(wiki.pages) + 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) - 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) + expect(slugs).to match_array(expected_slugs) + end end end - it 'returns an array sorted by alphabetical position' do - # Directories and pages within directories are sorted alphabetically. - # Pages at root come before everything. - expected_order = ['page_1', 'dir_1/page_2', 'dir_1/dir_1_1/page_3', - 'dir_2/page_4', 'dir_2/page_5'] + context 'sort by created_at' do + let(:grouped_entries) { described_class.group_by_directory(wiki.pages(sort: 'created_at')) } + let(:expected_grouped_entries) { [dir_1_1, page_1, dir_1, 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/page_4', 'dir_2/page_5', 'page_1', 'page_6'] grouped_entries = described_class.group_by_directory(wiki.pages)