Fix search sorting issue

During the testing of the original MR on staging we noticed that
sort doesn't work for all scopes. Turns out that we forgot to pass
sort parameter to some classes.
This MR fixes this problem and adds specs to catch similar type of
errors in the future.

app/services/search/group_service.rb

@@ -16,6 +16,7 @@ module Search
         params[:search],
         projects,
         group: group,
+        sort: params[:sort],
         filters: { state: params[:state], confidential: params[:confidential] }
       )
     end

app/services/search/project_service.rb

@@ -17,6 +17,7 @@ module Search
                                        params[:search],
                                        project: project,
                                        repository_ref: params[:repository_ref],
+                                       sort: params[:sort],
                                        filters: { confidential: params[:confidential], state: params[:state] }
                                       )
     end

ee/app/services/ee/search/group_service.rb

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

ee/app/services/ee/search/project_service.rb

@@ -15,6 +15,7 @@ module EE
           params[:search],
           project: project,
           repository_ref: repository_ref,
+          sort: params[:sort],
           filters: { confidential: params[:confidential], state: params[:state] }
         )
       end

ee/lib/gitlab/elastic/group_search_results.rb

@@ -8,12 +8,12 @@ module Gitlab
     class GroupSearchResults < Gitlab::Elastic::SearchResults
       attr_reader :group, :default_project_filter, :filters
 
-      def initialize(current_user, query, limit_project_ids = nil, group:, public_and_internal_projects: false, default_project_filter: false, filters: {})
+      def initialize(current_user, query, limit_project_ids = nil, group:, public_and_internal_projects: false, default_project_filter: false, sort: nil, filters: {})
         @group = group
         @default_project_filter = default_project_filter
         @filters = filters
 
-        super(current_user, query, limit_project_ids, public_and_internal_projects: public_and_internal_projects, filters: filters)
+        super(current_user, query, limit_project_ids, public_and_internal_projects: public_and_internal_projects, sort: sort, filters: filters)
       end
     end
   end

ee/lib/gitlab/elastic/project_search_results.rb

@@ -8,11 +8,11 @@ module Gitlab
     class ProjectSearchResults < Gitlab::Elastic::SearchResults
       attr_reader :project, :repository_ref, :filters
 
-      def initialize(current_user, query, project:, repository_ref: nil, filters: {})
+      def initialize(current_user, query, project:, repository_ref: nil, sort: nil, filters: {})
         @project = project
         @repository_ref = repository_ref.presence || project.default_branch
 
-        super(current_user, query, [project.id], public_and_internal_projects: false, filters: filters)
+        super(current_user, query, [project.id], public_and_internal_projects: false, sort: sort, filters: filters)
       end
 
       private

ee/lib/gitlab/elastic/search_results.rb

@@ -195,7 +195,7 @@ module Gitlab
 
       def issues
         strong_memoize(:issues) do
-          options = base_options.merge(filters.slice(:confidential, :state))
+          options = base_options.merge(filters.slice(:sort, :confidential, :state))
 
           Issue.elastic_search(query, options: options)
         end
@@ -216,8 +216,7 @@ module Gitlab
 
       def merge_requests
         strong_memoize(:merge_requests) do
-          options = base_options
-          options[:state] = filters[:state] if filters.key?(:state)
+          options = base_options.merge(filters.slice(:sort, :state))
 
           MergeRequest.elastic_search(query, options: options)
         end

ee/spec/services/search/global_service_spec.rb

@@ -79,25 +79,63 @@ RSpec.describe Search::GlobalService do
     end
 
     context 'issue' do
-      let!(:issue) { create :issue, project: project }
-      let!(:note) { create :note, project: project, noteable: issue }
+      let(:scope) { 'issues' }
 
-      where(:project_level, :feature_access_level, :membership, :expected_count) do
-        permission_table_for_guest_feature_access
+      context 'visibility' do
+        let!(:issue) { create :issue, project: project }
+        let!(:note) { create :note, project: project, noteable: issue }
+
+        where(:project_level, :feature_access_level, :membership, :expected_count) do
+          permission_table_for_guest_feature_access
+        end
+
+        with_them do
+          it "respects visibility" do
+            update_feature_access_level(project, feature_access_level)
+            ensure_elasticsearch_index!
+
+            expect_search_results(user, 'issues', expected_count: expected_count) do |user|
+              described_class.new(user, search: issue.title).execute
+            end
+
+            expect_search_results(user, 'notes', expected_count: expected_count) do |user|
+              described_class.new(user, search: note.note).execute
+            end
+          end
+        end
       end
 
-      with_them do
-        it "respects visibility" do
-          update_feature_access_level(project, feature_access_level)
+      context 'sort by created_at' do
+        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) }
+
+        before do
           ensure_elasticsearch_index!
+        end
 
-          expect_search_results(user, 'issues', expected_count: expected_count) do |user|
-            described_class.new(user, search: issue.title).execute
-          end
+        include_examples 'search results sorted' do
+          let(:results) { described_class.new(nil, search: 'sorted', sort: sort).execute }
+        end
+      end
+    end
 
-          expect_search_results(user, 'notes', expected_count: expected_count) do |user|
-            described_class.new(user, search: note.note).execute
-          end
+    context 'merge_request' do
+      let(:scope) { 'merge_requests' }
+
+      context 'sort by created_at' do
+        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) }
+
+        before do
+          ensure_elasticsearch_index!
+        end
+
+        include_examples 'search results sorted' do
+          let(:results) { described_class.new(nil, search: 'sorted', sort: sort).execute }
         end
       end
     end

ee/spec/services/search/group_service_spec.rb

@@ -233,6 +233,44 @@ RSpec.describe Search::GroupService, :elastic do
     end
   end
 
+  context 'issues' do
+    let(:scope) { 'issues' }
+
+    context 'sort by created_at' do
+      let!(:project) { create(:project, :public, group: group) }
+      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) }
+
+      before do
+        ensure_elasticsearch_index!
+      end
+
+      include_examples 'search results sorted' do
+        let(:results) { described_class.new(nil, group, search: 'sorted', sort: sort).execute }
+      end
+    end
+  end
+
+  context 'merge requests' do
+    let(:scope) { 'merge_requests' }
+
+    context 'sort by created_at' do
+      let!(:project) { create(:project, :public, group: group) }
+      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) }
+
+      before do
+        ensure_elasticsearch_index!
+      end
+
+      include_examples 'search results sorted' do
+        let(:results) { described_class.new(nil, group, search: 'sorted', sort: sort).execute }
+      end
+    end
+  end
+
   describe '#allowed_scopes' do
     context 'epics scope' do
       where(:feature_enabled, :epics_available, :epics_allowed) do

ee/spec/services/search/project_service_spec.rb

@@ -174,4 +174,42 @@ RSpec.describe Search::ProjectService do
       end
     end
   end
+
+  context 'issues' do
+    let(:scope) { 'issues' }
+
+    context 'sort by created_at', :elastic do
+      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) }
+
+      before do
+        ensure_elasticsearch_index!
+      end
+
+      include_examples 'search results sorted' do
+        let(:results) { described_class.new(project, nil, search: 'sorted', sort: sort).execute }
+      end
+    end
+  end
+
+  context 'merge requests' do
+    let(:scope) { 'merge_requests' }
+
+    context 'sort by created_at', :elastic do
+      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) }
+
+      before do
+        ensure_elasticsearch_index!
+      end
+
+      include_examples 'search results sorted' do
+        let(:results) { described_class.new(project, nil, search: 'sorted', sort: sort).execute }
+      end
+    end
+  end
 end

lib/gitlab/group_search_results.rb

@@ -4,10 +4,10 @@ module Gitlab
   class GroupSearchResults < SearchResults
     attr_reader :group
 
-    def initialize(current_user, query, limit_projects = nil, group:, default_project_filter: false, filters: {})
+    def initialize(current_user, query, limit_projects = nil, group:, default_project_filter: false, sort: nil, filters: {})
       @group = group
 
-      super(current_user, query, limit_projects, default_project_filter: default_project_filter, filters: filters)
+      super(current_user, query, limit_projects, default_project_filter: default_project_filter, sort: sort, filters: filters)
     end
 
     # rubocop:disable CodeReuse/ActiveRecord

lib/gitlab/project_search_results.rb

@@ -4,11 +4,11 @@ module Gitlab
   class ProjectSearchResults < SearchResults
     attr_reader :project, :repository_ref
 
-    def initialize(current_user, query, project:, repository_ref: nil, filters: {})
+    def initialize(current_user, query, project:, repository_ref: nil, sort: nil, filters: {})
       @project = project
       @repository_ref = repository_ref.presence
 
-      super(current_user, query, [project], filters: filters)
+      super(current_user, query, [project], sort: sort, filters: filters)
     end
 
     def objects(scope, page: nil, per_page: DEFAULT_PER_PAGE, preload_method: nil)

spec/services/search/global_service_spec.rb

@@ -52,4 +52,34 @@ RSpec.describe Search::GlobalService do
       end
     end
   end
+
+  context 'issues' do
+    let(:scope) { 'issues' }
+
+    context 'sort by created_at' do
+      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) }
+
+      include_examples 'search results sorted' do
+        let(:results) { described_class.new(nil, search: 'sorted', sort: sort).execute }
+      end
+    end
+  end
+
+  context 'merge_request' do
+    let(:scope) { 'merge_requests' }
+
+    context 'sort by created_at' do
+      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) }
+
+      include_examples 'search results sorted' do
+        let(:results) { described_class.new(nil, search: 'sorted', sort: sort).execute }
+      end
+    end
+  end
 end

spec/services/search/group_service_spec.rb

@@ -40,4 +40,36 @@ RSpec.describe Search::GroupService do
   describe 'basic search' do
     include_examples 'group search'
   end
+
+  context 'issues' do
+    let(:scope) { 'issues' }
+
+    context 'sort by created_at' do
+      let!(:group) { create(:group) }
+      let!(:project) { create(:project, :public, group: group) }
+      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' do
+        let(:results) { described_class.new(nil, group, search: 'sorted', sort: sort).execute }
+      end
+    end
+  end
+
+  context 'merge requests' do
+    let(:scope) { 'merge_requests' }
+
+    context 'sort by created_at' do
+      let!(:group) { create(:group) }
+      let!(:project) { create(:project, :public, group: group) }
+      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' do
+        let(:results) { described_class.new(nil, group, search: 'sorted', sort: sort).execute }
+      end
+    end
+  end
 end