Merge branch '255322-fix-search-sorting-issue' into 'master'

Fix search sorting issue

See merge request gitlab-org/gitlab!44009

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
       let(:allowed_scopes) { described_class.new(user, group, {}).allowed_scopes }

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