Merge branch '236465-search-results-sort-ui' into 'master'

Add the Ability to sort Issues and Merge Requests

See merge request gitlab-org/gitlab!45003

app/controllers/search_controller.rb

@@ -38,6 +38,7 @@ class SearchController < ApplicationController
     return if check_single_commit_result?
 
     @search_term = params[:search]
+    @sort = params[:sort] || default_sort
 
     @scope = search_service.scope
     @show_snippets = search_service.show_snippets?
@@ -81,6 +82,11 @@ class SearchController < ApplicationController
     SCOPE_PRELOAD_METHOD[@scope.to_sym]
   end
 
+  # overridden in EE
+  def default_sort
+    'created_desc'
+  end
+
   def search_term_valid?
     unless search_service.valid_query_length?
       flash[:alert] = t('errors.messages.search_chars_too_long', count: SearchService::SEARCH_CHAR_LIMIT)

app/helpers/sorting_helper.rb

@@ -28,7 +28,8 @@ module SortingHelper
       sort_value_contacted_date    => sort_title_contacted_date,
       sort_value_relative_position => sort_title_relative_position,
       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
 
@@ -81,6 +82,13 @@ module SortingHelper
     }
   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
     {
       sort_value_name             => sort_title_name,
@@ -218,6 +226,10 @@ module SortingHelper
     sort_options_hash[sort_value]
   end
 
+  def search_sort_option_title(sort_value)
+    sort_options_hash[sort_value]
+  end
+
   def sort_direction_icon(sort_value)
     case sort_value
     when sort_value_milestone, sort_value_due_date, /_asc\z/
@@ -256,6 +268,13 @@ module SortingHelper
     sort_direction_button(url, reverse_sort, sort_value)
   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.
   def sort_title_access_level_asc
     s_('SortOptions|Access level, ascending')
@@ -421,6 +440,10 @@ module SortingHelper
     s_('SortOptions|Expired date')
   end
 
+  def sort_title_relevant
+    s_('SortOptions|Relevant')
+  end
+
   # Values.
   def sort_value_access_level_asc
     'access_level_asc'
@@ -582,6 +605,10 @@ module SortingHelper
     'expired_asc'
   end
 
+  def sort_value_relevant
+    'relevant'
+  end
+
   def packages_sort_options_hash
     {
       sort_value_recently_created  => sort_title_created_date,

app/views/search/_results.html.haml

@@ -3,22 +3,26 @@
   = render partial: "search/results/empty"
   = render_if_exists 'shared/promotions/promote_advanced_search'
 - else
-  .row-content-block.d-md-flex.text-left.align-items-center
-    - unless @search_objects.is_a?(Kaminari::PaginatableWithoutCount)
-      = search_entries_info(@search_objects, @scope, @search_term)
-    - unless @show_snippets
-      - if @project
-        - link_to_project = link_to(@project.full_name, @project, class: 'ml-md-1')
-        - if @scope == 'blobs'
-          = s_("SearchCodeResults|in")
-          .mx-md-1
-            = render partial: "shared/ref_switcher", locals: { ref: repository_ref(@project), form_path: request.fullpath, field_name: 'repository_ref' }
-          = s_('SearchCodeResults|of %{link_to_project}').html_safe % { link_to_project: link_to_project }
-        - else
-          = _("in project %{link_to_project}").html_safe % { link_to_project: link_to_project }
-      - 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 }
+  .search-results-status
+    .row-content-block.gl-display-flex
+      .gl-display-md-flex.gl-text-left.gl-align-items-center.gl-flex-grow-1
+        - unless @search_objects.is_a?(Kaminari::PaginatableWithoutCount)
+          = search_entries_info(@search_objects, @scope, @search_term)
+        - unless @show_snippets
+          - if @project
+            - link_to_project = link_to(@project.full_name, @project, class: 'ml-md-1')
+            - if @scope == 'blobs'
+              = s_("SearchCodeResults|in")
+              .mx-md-1
+                = render partial: "shared/ref_switcher", locals: { ref: repository_ref(@project), form_path: request.fullpath, field_name: 'repository_ref' }
+              = s_('SearchCodeResults|of %{link_to_project}').html_safe % { link_to_project: link_to_project }
+            - else
+              = _("in project %{link_to_project}").html_safe % { link_to_project: link_to_project }
+          - 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 partial: "search/results/filters"
 

app/views/search/_sort_dropdown.html.haml

+- 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)

changelogs/unreleased/236465-search-results-sort-ui.yml

+---
+title: Add ability to sort search results for issues and merge requests
+merge_request: 45003
+author:
+type: added

ee/app/controllers/ee/search_controller.rb

@@ -3,6 +3,7 @@
 module EE
   module SearchController
     extend ActiveSupport::Concern
+    extend ::Gitlab::Utils::Override
 
     prepended do
       # track unique users of advanced global search
@@ -18,6 +19,15 @@ module EE
 
     private
 
+    override :default_sort
+    def default_sort
+      if search_service.use_elasticsearch?
+        'relevant'
+      else
+        super
+      end
+    end
+
     def track_search_advanced?
       search_service.use_elasticsearch?
     end

ee/app/views/search/_sort_by_relevancy.haml

+- if search_service.use_elasticsearch?
+  = sortable_item(sort_title_relevant, page_filter_path(sort: sort_value_relevant), sort_title)

ee/lib/elastic/latest/application_class_proxy.rb

@@ -136,13 +136,13 @@ module Elastic
 
       def apply_sort(query_hash, options)
         case options[:sort]
-        when 'oldest'
+        when 'created_asc'
           query_hash.merge(sort: {
             created_at: {
               order: 'asc'
             }
           })
-        when 'newest'
+        when 'created_desc'
           query_hash.merge(sort: {
             created_at: {
               order: 'desc'

ee/spec/views/search/_sort_dropdown.html.haml_spec.rb

+# 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

lib/gitlab/search_results.rb

@@ -129,12 +129,12 @@ module Gitlab
     # rubocop: disable CodeReuse/ActiveRecord
     def apply_sort(scope)
       case sort
-      when 'oldest'
+      when 'created_asc'
         scope.reorder('created_at ASC')
-      when 'newest'
+      when 'created_desc'
         scope.reorder('created_at DESC')
       else
-        scope
+        scope.reorder('created_at DESC')
       end
     end
     # rubocop: enable CodeReuse/ActiveRecord

locale/gitlab.pot

@@ -24949,6 +24949,9 @@ msgstr ""
 msgid "SortOptions|Recently starred"
 msgstr ""
 
+msgid "SortOptions|Relevant"
+msgstr ""
+
 msgid "SortOptions|Size"
 msgstr ""
 

spec/features/search/user_searches_for_issues_spec.rb

@@ -5,8 +5,8 @@ require 'spec_helper'
 RSpec.describe 'User searches for issues', :js do
   let(:user) { create(:user) }
   let(:project) { create(:project, namespace: user.namespace) }
-  let!(:issue1) { create(:issue, title: 'Foo', project: project) }
-  let!(:issue2) { create(:issue, :closed, :confidential, title: 'Bar', project: project) }
+  let!(:issue1) { create(:issue, title: 'issue Foo', project: project, created_at: 1.hour.ago) }
+  let!(:issue2) { create(:issue, :closed, :confidential, title: 'issue Bar', project: project) }
 
   def search_for_issue(search)
     fill_in('dashboard_search', with: search)
@@ -67,6 +67,22 @@ RSpec.describe 'User searches for issues', :js do
       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
       it 'finds an issue' do
         find('.js-search-project-dropdown').click

spec/helpers/sorting_helper_spec.rb

@@ -50,6 +50,24 @@ RSpec.describe SortingHelper do
     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)
     allow(helper.controller).to receive(:controller_path).and_return(value)
   end

spec/support/shared_examples/lib/gitlab/search_results_sorted_shared_examples.rb

@@ -2,7 +2,7 @@
 
 RSpec.shared_examples 'search results sorted' do
   context 'sort: newest' do
-    let(:sort) { 'newest' }
+    let(:sort) { 'created_desc' }
 
     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])
@@ -10,7 +10,7 @@ RSpec.shared_examples 'search results sorted' do
   end
 
   context 'sort: oldest' do
-    let(:sort) { 'oldest' }
+    let(:sort) { 'created_asc' }
 
     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])