Merge branch '255322-search-sort-issues-and-mrs' into 'master'

Add the Ability to Sort to Issue and Merge Request: Backend

Closes #255322

See merge request gitlab-org/gitlab!43295

app/helpers/search_helper.rb

 # frozen_string_literal: true
 
 module SearchHelper
-  SEARCH_PERMITTED_PARAMS = [:search, :scope, :project_id, :group_id, :repository_ref, :snippets, :state, :confidential].freeze
+  SEARCH_PERMITTED_PARAMS = [:search, :scope, :project_id, :group_id, :repository_ref, :snippets, :sort, :state, :confidential].freeze
 
   def search_autocomplete_opts(term)
     return unless current_user

app/services/search/global_service.rb

@@ -14,6 +14,7 @@ module Search
       Gitlab::SearchResults.new(current_user,
                                 params[:search],
                                 projects,
+                                sort: params[:sort],
                                 filters: { state: params[:state], confidential: params[:confidential] })
     end
 

changelogs/unreleased/255322-search-sort-issues-and-mrs.yml

+---
+title: Add sort parameter to Issue and Merge Request scopes
+merge_request: 43295
+author:
+type: added

ee/app/services/ee/search/global_service.rb

@@ -16,6 +16,7 @@ module EE
           params[:search],
           elastic_projects,
           public_and_internal_projects: elastic_global,
+          sort: params[:sort],
           filters: { confidential: params[:confidential], state: params[:state] }
         )
       end

ee/lib/elastic/latest/application_class_proxy.rb

@@ -118,6 +118,25 @@ module Elastic
         query_hash
       end
 
+      def apply_sort(query_hash, options)
+        case options[:sort]
+        when 'oldest'
+          query_hash.merge(sort: {
+            created_at: {
+              order: 'asc'
+            }
+          })
+        when 'newest'
+          query_hash.merge(sort: {
+            created_at: {
+              order: 'desc'
+            }
+          })
+        else
+          query_hash
+        end
+      end
+
       # Builds an elasticsearch query that will select projects the user is
       # granted access to.
       #

ee/lib/elastic/latest/issue_class_proxy.rb

@@ -24,6 +24,7 @@ module Elastic
         query_hash = project_ids_filter(query_hash, options)
         query_hash = confidentiality_filter(query_hash, options)
         query_hash = state_filter(query_hash, options)
+        query_hash = apply_sort(query_hash, options)
 
         search(query_hash, options)
       end

ee/lib/elastic/latest/merge_request_class_proxy.rb

@@ -23,6 +23,7 @@ module Elastic
         options[:features] = 'merge_requests'
         query_hash = project_ids_filter(query_hash, options)
         query_hash = state_filter(query_hash, options)
+        query_hash = apply_sort(query_hash, options)
 
         search(query_hash, options)
       end

ee/lib/gitlab/elastic/search_results.rb

@@ -7,17 +7,18 @@ module Gitlab
 
       DEFAULT_PER_PAGE = Gitlab::SearchResults::DEFAULT_PER_PAGE
 
-      attr_reader :current_user, :query, :public_and_internal_projects, :filters
+      attr_reader :current_user, :query, :public_and_internal_projects, :sort, :filters
 
       # Limit search results by passed projects
       # It allows us to search only for projects user has access to
       attr_reader :limit_project_ids
 
-      def initialize(current_user, query, limit_project_ids = nil, public_and_internal_projects: true, filters: {})
+      def initialize(current_user, query, limit_project_ids = nil, public_and_internal_projects: true, sort: nil, filters: {})
         @current_user = current_user
         @query = query
         @limit_project_ids = limit_project_ids
         @public_and_internal_projects = public_and_internal_projects
+        @sort = sort
         @filters = filters
       end
 
@@ -170,7 +171,7 @@ module Gitlab
         relation = relation.public_send(preload_method) if preload_method # rubocop:disable GitlabSecurity/PublicSend
 
         Kaminari.paginate_array(
-          relation,
+          relation.to_a,
           total_count: paginated_base.total_count,
           limit: per_page,
           offset: per_page * (page - 1)
@@ -181,7 +182,8 @@ module Gitlab
         {
           current_user: current_user,
           project_ids: limit_project_ids,
-          public_and_internal_projects: public_and_internal_projects
+          public_and_internal_projects: public_and_internal_projects,
+          sort: sort
         }
       end
 

ee/spec/lib/gitlab/elastic/search_results_spec.rb

@@ -158,6 +158,8 @@ RSpec.describe Gitlab::Elastic::SearchResults, :elastic, :sidekiq_might_not_need
   end
 
   describe 'issues' do
+    let(:scope) { 'issues' }
+
     before do
       @issue_1 = create(
         :issue,
@@ -228,7 +230,6 @@ RSpec.describe Gitlab::Elastic::SearchResults, :elastic, :sidekiq_might_not_need
       let!(:opened_result) { create(:issue, :opened, project: project, title: 'foo opened') }
       let!(:confidential_result) { create(:issue, :confidential, project: project, title: 'foo confidential') }
 
-      let(:scope) { 'issues' }
       let(:results) { described_class.new(user, 'foo', [project.id], filters: filters) }
 
       before do
@@ -240,6 +241,22 @@ RSpec.describe Gitlab::Elastic::SearchResults, :elastic, :sidekiq_might_not_need
       include_examples 'search results filtered by state'
       include_examples 'search results filtered by confidential'
     end
+
+    context 'ordering' do
+      let(:query) { 'sorted' }
+      let!(:project) { create(:project, :public) }
+      let!(:old_result) { create(:issue, project: project, title: 'sorted old', created_at: 1.month.ago) }
+      let!(:new_result) { create(:issue, project: project, title: 'sorted recent', created_at: 1.day.ago) }
+      let!(:very_old_result) { create(:issue, project: project, title: 'sorted very old', created_at: 1.year.ago) }
+
+      let(:results) { described_class.new(user, query, [project.id], sort: sort) }
+
+      before do
+        ensure_elasticsearch_index!
+      end
+
+      include_examples 'search results sorted'
+    end
   end
 
   describe 'notes' do
@@ -481,6 +498,8 @@ RSpec.describe Gitlab::Elastic::SearchResults, :elastic, :sidekiq_might_not_need
   end
 
   describe 'merge requests' do
+    let(:scope) { 'merge_requests' }
+
     before do
       @merge_request_1 = create(
         :merge_request,
@@ -554,7 +573,6 @@ RSpec.describe Gitlab::Elastic::SearchResults, :elastic, :sidekiq_might_not_need
       let!(:opened_result) { create(:merge_request, :opened, source_project: project, title: 'foo opened') }
       let!(:closed_result) { create(:merge_request, :closed, source_project: project, title: 'foo closed') }
 
-      let(:scope) { 'merge_requests' }
       let(:results) { described_class.new(user, 'foo', [project.id], filters: filters) }
 
       include_examples 'search results filtered by state' do
@@ -563,6 +581,22 @@ RSpec.describe Gitlab::Elastic::SearchResults, :elastic, :sidekiq_might_not_need
         end
       end
     end
+
+    context 'ordering' do
+      let(:query) { 'sorted' }
+      let!(:project) { create(:project, :public) }
+      let!(:old_result) { create(:merge_request, :opened, source_project: project, source_branch: 'old-1', title: 'sorted old', created_at: 1.month.ago) }
+      let!(:new_result) { create(:merge_request, :opened, source_project: project, source_branch: 'new-1', title: 'sorted recent', created_at: 1.day.ago) }
+      let!(:very_old_result) { create(:merge_request, :opened, source_project: project, source_branch: 'very-old-1', title: 'sorted very old', created_at: 1.year.ago) }
+
+      let(:results) { described_class.new(user, query, [project.id], sort: sort) }
+
+      before do
+        ensure_elasticsearch_index!
+      end
+
+      include_examples 'search results sorted'
+    end
   end
 
   describe 'project scoping' do

lib/gitlab/search_results.rb

@@ -7,7 +7,7 @@ module Gitlab
     DEFAULT_PAGE = 1
     DEFAULT_PER_PAGE = 20
 
-    attr_reader :current_user, :query, :filters
+    attr_reader :current_user, :query, :sort, :filters
 
     # Limit search results by passed projects
     # It allows us to search only for projects user has access to
@@ -19,11 +19,12 @@ module Gitlab
     # query
     attr_reader :default_project_filter
 
-    def initialize(current_user, query, limit_projects = nil, default_project_filter: false, filters: {})
+    def initialize(current_user, query, limit_projects = nil, sort: nil, default_project_filter: false, filters: {})
       @current_user = current_user
       @query = query
       @limit_projects = limit_projects || Project.all
       @default_project_filter = default_project_filter
+      @sort = sort
       @filters = filters
     end
 
@@ -124,6 +125,19 @@ module Gitlab
       end
     end
 
+    # rubocop: disable CodeReuse/ActiveRecord
+    def apply_sort(scope)
+      case sort
+      when 'oldest'
+        scope.reorder('created_at ASC')
+      when 'newest'
+        scope.reorder('created_at DESC')
+      else
+        scope
+      end
+    end
+    # rubocop: enable CodeReuse/ActiveRecord
+
     def projects
       limit_projects.search(query)
     end
@@ -135,7 +149,7 @@ module Gitlab
         issues = issues.where(project_id: project_ids_relation) # rubocop: disable CodeReuse/ActiveRecord
       end
 
-      issues
+      apply_sort(issues)
     end
 
     # rubocop: disable CodeReuse/ActiveRecord
@@ -155,7 +169,7 @@ module Gitlab
         merge_requests = merge_requests.in_projects(project_ids_relation)
       end
 
-      merge_requests
+      apply_sort(merge_requests)
     end
 
     def default_scope

spec/lib/gitlab/search_results_spec.rb

@@ -11,9 +11,11 @@ RSpec.describe Gitlab::SearchResults do
   let_it_be(:issue) { create(:issue, project: project, title: 'foo') }
   let_it_be(:milestone) { create(:milestone, project: project, title: 'foo') }
   let(:merge_request) { create(:merge_request, source_project: project, title: 'foo') }
+  let(:query) { 'foo' }
   let(:filters) { {} }
+  let(:sort) { nil }
 
-  subject(:results) { described_class.new(user, 'foo', Project.order(:id), filters: filters) }
+  subject(:results) { described_class.new(user, query, Project.order(:id), sort: sort, filters: filters) }
 
   context 'as a user with access' do
     before do
@@ -137,10 +139,12 @@ RSpec.describe Gitlab::SearchResults do
     end
 
     describe '#merge_requests' do
+      let(:scope) { 'merge_requests' }
+
       it 'includes project filter by default' do
         expect(results).to receive(:project_ids_relation).and_call_original
 
-        results.objects('merge_requests')
+        results.objects(scope)
       end
 
       it 'skips project filter if default project context is used' do
@@ -148,24 +152,34 @@ RSpec.describe Gitlab::SearchResults do
 
         expect(results).not_to receive(:project_ids_relation)
 
-        results.objects('merge_requests')
+        results.objects(scope)
       end
 
       context 'filtering' do
         let!(:opened_result) { create(:merge_request, :opened, source_project: project, title: 'foo opened') }
         let!(:closed_result) { create(:merge_request, :closed, source_project: project, title: 'foo closed') }
-        let(:scope) { 'merge_requests' }
         let(:query) { 'foo' }
 
         include_examples 'search results filtered by state'
       end
+
+      context 'ordering' do
+        let(:query) { 'sorted' }
+        let!(:old_result) { create(:merge_request, :opened, source_project: project, source_branch: 'old-1', title: 'sorted old', created_at: 1.month.ago) }
+        let!(:new_result) { create(:merge_request, :opened, source_project: project, source_branch: 'new-1', title: 'sorted recent', created_at: 1.day.ago) }
+        let!(:very_old_result) { create(:merge_request, :opened, source_project: project, source_branch: 'very-old-1', title: 'sorted very old', created_at: 1.year.ago) }
+
+        include_examples 'search results sorted'
+      end
     end
 
     describe '#issues' do
+      let(:scope) { 'issues' }
+
       it 'includes project filter by default' do
         expect(results).to receive(:project_ids_relation).and_call_original
 
-        results.objects('issues')
+        results.objects(scope)
       end
 
       it 'skips project filter if default project context is used' do
@@ -173,12 +187,10 @@ RSpec.describe Gitlab::SearchResults do
 
         expect(results).not_to receive(:project_ids_relation)
 
-        results.objects('issues')
+        results.objects(scope)
       end
 
       context 'filtering' do
-        let(:scope) { 'issues' }
-
         let_it_be(:closed_result) { create(:issue, :closed, project: project, title: 'foo closed') }
         let_it_be(:opened_result) { create(:issue, :opened, project: project, title: 'foo open') }
         let_it_be(:confidential_result) { create(:issue, :confidential, project: project, title: 'foo confidential') }
@@ -186,6 +198,15 @@ RSpec.describe Gitlab::SearchResults do
         include_examples 'search results filtered by state'
         include_examples 'search results filtered by confidential'
       end
+
+      context 'ordering' do
+        let(:query) { 'sorted' }
+        let!(:old_result) { create(:issue, project: project, title: 'sorted old', created_at: 1.month.ago) }
+        let!(:new_result) { create(:issue, project: project, title: 'sorted recent', created_at: 1.day.ago) }
+        let!(:very_old_result) { create(:issue, project: project, title: 'sorted very old', created_at: 1.year.ago) }
+
+        include_examples 'search results sorted'
+      end
     end
 
     describe '#users' do

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

+# frozen_string_literal: true
+
+RSpec.shared_examples 'search results sorted' do
+  context 'sort: newest' do
+    let(:sort) { 'newest' }
+
+    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])
+    end
+  end
+
+  context 'sort: oldest' do
+    let(:sort) { 'oldest' }
+
+    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])
+    end
+  end
+end