Merge branch '229982-fix-in-page-sort-order' into 'master'

Fix Elasticsearch sorting incorrectly from DB

See merge request gitlab-org/gitlab!37665

config/initializers/elastic_client_setup.rb

@@ -13,6 +13,7 @@ Gitlab.ee do
   Elasticsearch::Model::Adapter::Multiple::Records.prepend GemExtensions::Elasticsearch::Model::Adapter::Multiple::Records
   Elasticsearch::Model::Indexing::InstanceMethods.prepend GemExtensions::Elasticsearch::Model::Indexing::InstanceMethods
   Elasticsearch::Model::Adapter::ActiveRecord::Importing.prepend GemExtensions::Elasticsearch::Model::Adapter::ActiveRecord::Importing
+  Elasticsearch::Model::Adapter::ActiveRecord::Records.prepend GemExtensions::Elasticsearch::Model::Adapter::ActiveRecord::Records
   Elasticsearch::Model::Client::InstanceMethods.prepend GemExtensions::Elasticsearch::Model::Client
   Elasticsearch::Model::Client::ClassMethods.prepend GemExtensions::Elasticsearch::Model::Client
   Elasticsearch::Model::ClassMethods.prepend GemExtensions::Elasticsearch::Model::Client

ee/changelogs/unreleased/229982-fix-in-page-sort-order.yml

+---
+title: Fix Elasticsearch sorting incorrectly from DB
+merge_request: 37665
+author:
+type: fixed

ee/lib/gem_extensions/elasticsearch/model/adapter/active_record/records.rb

+# frozen_string_literal: true
+
+module GemExtensions
+  module Elasticsearch
+    module Model
+      module Adapter
+        module ActiveRecord
+          # rubocop:disable all
+          module Records
+            # Original method
+            # https://github.com/elastic/elasticsearch-rails/blob/v6.1.0/elasticsearch-model/lib/elasticsearch/model/adapters/active_record.rb#L21
+            def records
+              sql_records = klass.where(klass.primary_key => ids)
+              sql_records = sql_records.includes(self.options[:includes]) if self.options[:includes]
+
+              # Re-order records based on the order from Elasticsearch hits
+              # by redefining `to_a`, unless the user has called `order()`
+              #
+              sql_records.instance_exec(response.response['hits']['hits']) do |hits|
+                ar_records_method_name = :to_a
+                ar_records_method_name = :records if defined?(::ActiveRecord) && ::ActiveRecord::VERSION::MAJOR >= 5
+
+                define_singleton_method(ar_records_method_name) do
+                  if defined?(::ActiveRecord) && ::ActiveRecord::VERSION::MAJOR >= 4
+                    self.load
+                  else
+                    self.__send__(:exec_queries)
+                  end
+                  if !self.order_values.present?
+                    # BEGIN_MONKEY_PATCH
+                    #
+                    # Monkey patch sorting because we monkey patch ids and the
+                    # previous code uses `hit['_id']` which does not match our
+                    # record.id. We instead need `hit['_source']['id']`.
+                    @records.sort_by { |record| hits.index { |hit| hit['_source']['id'].to_s == record.id.to_s } }
+                    # END_MONKEY_PATCH
+                  else
+                    @records
+                  end
+                end if self
+              end
+
+              sql_records
+            end
+          end
+          # rubocop:enable all
+        end
+      end
+    end
+  end
+end

ee/spec/lib/gem_extensions/elasticsearch/model/adapter/active_record/records_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Elasticsearch::Model::Adapter::ActiveRecord::Records, :elastic do
+  describe '#records' do
+    let(:user) { create(:user) }
+    let(:search_options) { { options: { current_user: user, project_ids: :any } } }
+
+    before do
+      stub_ee_application_setting(elasticsearch_indexing: true)
+
+      @middle_relevant = create(
+        :issue,
+        title: 'Sorting could improve', # Some keywords in title
+        description: 'I think you could make it better'
+      )
+      @least_relevant = create(
+        :issue,
+        title: 'I love GitLab', # No keywords in title
+        description: 'There is so much to love! For example, you could not possibly make sorting any better'
+      )
+
+      @most_relevant = create(
+        :issue,
+        title: 'Make sorting better', # All keywords in title
+        description: 'This issue is here to make the sorting better'
+      )
+
+      ensure_elasticsearch_index!
+    end
+
+    it 'returns results in the same sorted order as they come back from Elasticsearch' do
+      expect(Issue.elastic_search('make sorting better', search_options).records.to_a).to eq([
+        @most_relevant,
+        @middle_relevant,
+        @least_relevant
+      ])
+    end
+  end
+end