[MODEL] Ensure that specified ActiveRecord order is not overwritten by Elasticsearch search results order (#835)

* [MODEL] Ensure that specified ActiveRecord order is not overwritten by Elasticsearch query order

* [MODEL] Remove interception of #order method, as ordering is handled by checking order_values in #to_a

Author: estolfo

elasticsearch-model/lib/elasticsearch/model/adapters/active_record.rb

@@ -35,7 +35,11 @@ def records
                 else
                   self.__send__(:exec_queries)
                 end
-                @records.sort_by { |record| hits.index { |hit| hit['_id'].to_s == record.id.to_s } }
+                if !self.order_values.present?
+                  @records.sort_by { |record| hits.index { |hit| hit['_id'].to_s == record.id.to_s } }
+                else
+                  @records
+                end
               end if self
             end
 
@@ -47,30 +51,6 @@ def records
           def load
             records.__send__(:load)
           end
-
-          # Intercept call to the `order` method, so we can ignore the order from Elasticsearch
-          #
-          def order(*args)
-            sql_records = records.__send__ :order, *args
-
-            # Redefine the `to_a` method to the original one
-            #
-            sql_records.instance_exec do
-              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
-                @records
-              end
-            end
-
-            sql_records
-          end
         end
 
         module Callbacks

elasticsearch-model/spec/elasticsearch/model/adapters/active_record_spec.rb

@@ -66,6 +66,7 @@ class DummyClassForActiveRecord; end
     let(:instance) do
       model.tap do |inst|
         allow(inst).to receive(:klass).and_return(double('class', primary_key: :some_key, where: records)).at_least(:once)
+        allow(inst).to receive(:order).and_return(double('class', primary_key: :some_key, where: records)).at_least(:once)
       end
     end
 
@@ -95,6 +96,7 @@ class DummyClassForActiveRecord; end
 
         before do
           records.instance_variable_set(:@records, records)
+          allow(records).to receive(:order_values).and_return([])
         end
 
         it 'reorders the records based on hits order' do
@@ -109,14 +111,12 @@ class DummyClassForActiveRecord; end
       context 'when the records have a different order than the hits' do
 
         before do
-          records.instance_variable_set(:@records, records)
-          expect(instance.records).to receive(:order).and_return(records)
+          records.instance_variable_set(:@records, [record_2, record_1])
+          allow(records).to receive(:order_values).and_return([double('order_definition')])
         end
 
         it 'reorders the records based on hits order' do
-          expect(records.collect(&:id)).to eq([1, 2])
           expect(instance.records.to_a.collect(&:id)).to eq([2, 1])
-          expect(instance.order(:foo).to_a.collect(&:id)).to eq([1, 2])
         end
       end
     end

elasticsearch-model/test/integration/active_record_basic_test.rb

@@ -231,6 +231,13 @@ def as_indexed_json(options = {})
           end
         end
 
+        should "allow ordering following any method chain in SQL" do
+          if defined?(::ActiveRecord) && ::ActiveRecord::VERSION::MAJOR >= 4
+            response = Article.search query: { match: { title: { query: 'test' } } }
+            assert_equal 'Testing Coding', response.records.distinct.order(id: :desc).first.title
+          end
+        end
+
         should "allow dot access to response" do
           response = Article.search query: { match: { title: { query: 'test' } } },
                                     aggregations: {