Commit 6e4252ed authored by Dmitry Gruzd's avatar Dmitry Gruzd Committed by Natalia Tepluhina

Add ability to sort issues and merge_requests: UI

When searching for Issues and Merge Request sometimes you don't want to
see the most accurate result. You might want to see the the newest
result.
Now you can change from Relevant results to newest or oldest results. As
we add more filtering in the future this will be even more valuable.
parent 6c73102e
...@@ -36,6 +36,7 @@ class SearchController < ApplicationController ...@@ -36,6 +36,7 @@ class SearchController < ApplicationController
return unless search_term_valid? return unless search_term_valid?
@search_term = params[:search] @search_term = params[:search]
@sort = params[:sort] || default_sort
@scope = search_service.scope @scope = search_service.scope
@show_snippets = search_service.show_snippets? @show_snippets = search_service.show_snippets?
...@@ -81,6 +82,11 @@ class SearchController < ApplicationController ...@@ -81,6 +82,11 @@ class SearchController < ApplicationController
SCOPE_PRELOAD_METHOD[@scope.to_sym] SCOPE_PRELOAD_METHOD[@scope.to_sym]
end end
# overridden in EE
def default_sort
'created_desc'
end
def search_term_valid? def search_term_valid?
unless search_service.valid_query_length? unless search_service.valid_query_length?
flash[:alert] = t('errors.messages.search_chars_too_long', count: SearchService::SEARCH_CHAR_LIMIT) flash[:alert] = t('errors.messages.search_chars_too_long', count: SearchService::SEARCH_CHAR_LIMIT)
......
...@@ -28,7 +28,8 @@ module SortingHelper ...@@ -28,7 +28,8 @@ module SortingHelper
sort_value_contacted_date => sort_title_contacted_date, sort_value_contacted_date => sort_title_contacted_date,
sort_value_relative_position => sort_title_relative_position, sort_value_relative_position => sort_title_relative_position,
sort_value_size => sort_title_size, sort_value_size => sort_title_size,
sort_value_expire_date => sort_title_expire_date sort_value_expire_date => sort_title_expire_date,
sort_value_relevant => sort_title_relevant
} }
end end
...@@ -81,6 +82,13 @@ module SortingHelper ...@@ -81,6 +82,13 @@ module SortingHelper
} }
end end
def search_reverse_sort_options_hash
{
sort_value_recently_created => sort_value_oldest_created,
sort_value_oldest_created => sort_value_recently_created
}
end
def groups_sort_options_hash def groups_sort_options_hash
{ {
sort_value_name => sort_title_name, sort_value_name => sort_title_name,
...@@ -218,6 +226,10 @@ module SortingHelper ...@@ -218,6 +226,10 @@ module SortingHelper
sort_options_hash[sort_value] sort_options_hash[sort_value]
end end
def search_sort_option_title(sort_value)
sort_options_hash[sort_value]
end
def sort_direction_icon(sort_value) def sort_direction_icon(sort_value)
case sort_value case sort_value
when sort_value_milestone, sort_value_due_date, /_asc\z/ when sort_value_milestone, sort_value_due_date, /_asc\z/
...@@ -256,6 +268,13 @@ module SortingHelper ...@@ -256,6 +268,13 @@ module SortingHelper
sort_direction_button(url, reverse_sort, sort_value) sort_direction_button(url, reverse_sort, sort_value)
end end
def search_sort_direction_button(sort_value)
reverse_sort = search_reverse_sort_options_hash[sort_value]
url = page_filter_path(sort: reverse_sort)
sort_direction_button(url, reverse_sort, sort_value)
end
# Titles. # Titles.
def sort_title_access_level_asc def sort_title_access_level_asc
s_('SortOptions|Access level, ascending') s_('SortOptions|Access level, ascending')
...@@ -421,6 +440,10 @@ module SortingHelper ...@@ -421,6 +440,10 @@ module SortingHelper
s_('SortOptions|Expired date') s_('SortOptions|Expired date')
end end
def sort_title_relevant
s_('SortOptions|Relevant')
end
# Values. # Values.
def sort_value_access_level_asc def sort_value_access_level_asc
'access_level_asc' 'access_level_asc'
...@@ -582,6 +605,10 @@ module SortingHelper ...@@ -582,6 +605,10 @@ module SortingHelper
'expired_asc' 'expired_asc'
end end
def sort_value_relevant
'relevant'
end
def packages_sort_options_hash def packages_sort_options_hash
{ {
sort_value_recently_created => sort_title_created_date, sort_value_recently_created => sort_title_created_date,
......
...@@ -3,22 +3,26 @@ ...@@ -3,22 +3,26 @@
= render partial: "search/results/empty" = render partial: "search/results/empty"
= render_if_exists 'shared/promotions/promote_advanced_search' = render_if_exists 'shared/promotions/promote_advanced_search'
- else - else
.row-content-block.d-md-flex.text-left.align-items-center .search-results-status
- unless @search_objects.is_a?(Kaminari::PaginatableWithoutCount) .row-content-block.gl-display-flex
= search_entries_info(@search_objects, @scope, @search_term) .gl-display-md-flex.gl-text-left.gl-align-items-center.gl-flex-grow-1
- unless @show_snippets - unless @search_objects.is_a?(Kaminari::PaginatableWithoutCount)
- if @project = search_entries_info(@search_objects, @scope, @search_term)
- link_to_project = link_to(@project.full_name, @project, class: 'ml-md-1') - unless @show_snippets
- if @scope == 'blobs' - if @project
= s_("SearchCodeResults|in") - link_to_project = link_to(@project.full_name, @project, class: 'ml-md-1')
.mx-md-1 - if @scope == 'blobs'
= render partial: "shared/ref_switcher", locals: { ref: repository_ref(@project), form_path: request.fullpath, field_name: 'repository_ref' } = s_("SearchCodeResults|in")
= s_('SearchCodeResults|of %{link_to_project}').html_safe % { link_to_project: link_to_project } .mx-md-1
- else = render partial: "shared/ref_switcher", locals: { ref: repository_ref(@project), form_path: request.fullpath, field_name: 'repository_ref' }
= _("in project %{link_to_project}").html_safe % { link_to_project: link_to_project } = s_('SearchCodeResults|of %{link_to_project}').html_safe % { link_to_project: link_to_project }
- elsif @group - else
- link_to_group = link_to(@group.name, @group, class: 'ml-md-1') = _("in project %{link_to_project}").html_safe % { link_to_project: link_to_project }
= _("in group %{link_to_group}").html_safe % { link_to_group: link_to_group } - elsif @group
- link_to_group = link_to(@group.name, @group, class: 'ml-md-1')
= _("in group %{link_to_group}").html_safe % { link_to_group: link_to_group }
.gl-display-md-flex.gl-flex-direction-column
= render partial: 'search/sort_dropdown'
= render_if_exists 'shared/promotions/promote_advanced_search' = render_if_exists 'shared/promotions/promote_advanced_search'
= render partial: "search/results/filters" = render partial: "search/results/filters"
......
- return unless ['issues', 'merge_requests'].include?(@scope)
- sort_value = @sort
- sort_title = search_sort_option_title(sort_value)
.dropdown.gl-display-inline-block.gl-ml-3.filter-dropdown-container
.btn-group{ role: 'group' }
.btn-group{ role: 'group' }
%button.dropdown-menu-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
= render_if_exists('search/sort_by_relevancy', sort_title: sort_title)
= sortable_item(sort_title_recently_created, page_filter_path(sort: sort_value_recently_created), sort_title)
= search_sort_direction_button(sort_value)
---
title: Add ability to sort search results for issues and merge requests
merge_request: 45003
author:
type: added
...@@ -3,6 +3,7 @@ ...@@ -3,6 +3,7 @@
module EE module EE
module SearchController module SearchController
extend ActiveSupport::Concern extend ActiveSupport::Concern
extend ::Gitlab::Utils::Override
prepended do prepended do
# track unique users of advanced global search # track unique users of advanced global search
...@@ -18,6 +19,15 @@ module EE ...@@ -18,6 +19,15 @@ module EE
private private
override :default_sort
def default_sort
if search_service.use_elasticsearch?
'relevant'
else
super
end
end
def track_search_advanced? def track_search_advanced?
search_service.use_elasticsearch? search_service.use_elasticsearch?
end end
......
- if search_service.use_elasticsearch?
= sortable_item(sort_title_relevant, page_filter_path(sort: sort_value_relevant), sort_title)
...@@ -136,13 +136,13 @@ module Elastic ...@@ -136,13 +136,13 @@ module Elastic
def apply_sort(query_hash, options) def apply_sort(query_hash, options)
case options[:sort] case options[:sort]
when 'oldest' when 'created_asc'
query_hash.merge(sort: { query_hash.merge(sort: {
created_at: { created_at: {
order: 'asc' order: 'asc'
} }
}) })
when 'newest' when 'created_desc'
query_hash.merge(sort: { query_hash.merge(sort: {
created_at: { created_at: {
order: 'desc' order: 'desc'
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe 'search/_sort_dropdown' do
context 'when the search page is opened' do
before do
@scope = 'issues'
end
context 'with advanced search' do
before do
@search_service = instance_double(SearchService, use_elasticsearch?: true)
end
it 'displays the correct sort elements' do
render
expect(rendered).to have_selector('a', text: 'Relevant')
expect(rendered).to have_selector('a', text: 'Last created')
end
end
context 'without advanced search' do
before do
@search_service = instance_double(SearchService, use_elasticsearch?: false)
end
it 'displays the correct sort elements' do
render
expect(rendered).not_to have_selector('a', text: 'Relevant')
expect(rendered).to have_selector('a', text: 'Last created')
end
end
end
end
...@@ -133,12 +133,12 @@ module Gitlab ...@@ -133,12 +133,12 @@ module Gitlab
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
def apply_sort(scope) def apply_sort(scope)
case sort case sort
when 'oldest' when 'created_asc'
scope.reorder('created_at ASC') scope.reorder('created_at ASC')
when 'newest' when 'created_desc'
scope.reorder('created_at DESC') scope.reorder('created_at DESC')
else else
scope scope.reorder('created_at DESC')
end end
end end
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
......
...@@ -24766,6 +24766,9 @@ msgstr "" ...@@ -24766,6 +24766,9 @@ msgstr ""
msgid "SortOptions|Recently starred" msgid "SortOptions|Recently starred"
msgstr "" msgstr ""
msgid "SortOptions|Relevant"
msgstr ""
msgid "SortOptions|Size" msgid "SortOptions|Size"
msgstr "" msgstr ""
......
...@@ -5,8 +5,8 @@ require 'spec_helper' ...@@ -5,8 +5,8 @@ require 'spec_helper'
RSpec.describe 'User searches for issues', :js do RSpec.describe 'User searches for issues', :js do
let(:user) { create(:user) } let(:user) { create(:user) }
let(:project) { create(:project, namespace: user.namespace) } let(:project) { create(:project, namespace: user.namespace) }
let!(:issue1) { create(:issue, title: 'Foo', project: project) } let!(:issue1) { create(:issue, title: 'issue Foo', project: project, created_at: 1.hour.ago) }
let!(:issue2) { create(:issue, :closed, :confidential, title: 'Bar', project: project) } let!(:issue2) { create(:issue, :closed, :confidential, title: 'issue Bar', project: project) }
def search_for_issue(search) def search_for_issue(search)
fill_in('dashboard_search', with: search) fill_in('dashboard_search', with: search)
...@@ -67,6 +67,22 @@ RSpec.describe 'User searches for issues', :js do ...@@ -67,6 +67,22 @@ RSpec.describe 'User searches for issues', :js do
end end
end end
it 'sorts by created date' do
search_for_issue('issue')
page.within('.results') do
expect(page.all('.search-result-row').first).to have_link(issue2.title)
expect(page.all('.search-result-row').last).to have_link(issue1.title)
end
find('.reverse-sort-btn').click
page.within('.results') do
expect(page.all('.search-result-row').first).to have_link(issue1.title)
expect(page.all('.search-result-row').last).to have_link(issue2.title)
end
end
context 'when on a project page' do context 'when on a project page' do
it 'finds an issue' do it 'finds an issue' do
find('.js-search-project-dropdown').click find('.js-search-project-dropdown').click
......
...@@ -50,6 +50,24 @@ RSpec.describe SortingHelper do ...@@ -50,6 +50,24 @@ RSpec.describe SortingHelper do
end end
end end
describe '#search_sort_direction_button' do
before do
set_sorting_url 'test_label'
end
it 'keeps label filter param' do
expect(search_sort_direction_button('created_asc')).to include('label_name=test_label')
end
it 'returns icon with sort-lowest when sort is asc' do
expect(search_sort_direction_button('created_asc')).to include('sort-lowest')
end
it 'returns icon with sort-highest when sort is desc' do
expect(search_sort_direction_button('created_desc')).to include('sort-highest')
end
end
def stub_controller_path(value) def stub_controller_path(value)
allow(helper.controller).to receive(:controller_path).and_return(value) allow(helper.controller).to receive(:controller_path).and_return(value)
end end
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
RSpec.shared_examples 'search results sorted' do RSpec.shared_examples 'search results sorted' do
context 'sort: newest' do context 'sort: newest' do
let(:sort) { 'newest' } let(:sort) { 'created_desc' }
it 'sorts results by created_at' do it 'sorts results by created_at' do
expect(results.objects(scope).map(&:id)).to eq([new_result.id, old_result.id, very_old_result.id]) expect(results.objects(scope).map(&:id)).to eq([new_result.id, old_result.id, very_old_result.id])
...@@ -10,7 +10,7 @@ RSpec.shared_examples 'search results sorted' do ...@@ -10,7 +10,7 @@ RSpec.shared_examples 'search results sorted' do
end end
context 'sort: oldest' do context 'sort: oldest' do
let(:sort) { 'oldest' } let(:sort) { 'created_asc' }
it 'sorts results by created_at' do it 'sorts results by created_at' do
expect(results.objects(scope).map(&:id)).to eq([very_old_result.id, old_result.id, new_result.id]) expect(results.objects(scope).map(&:id)).to eq([very_old_result.id, old_result.id, new_result.id])
......
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