Merge pull request #1052 from cerebris/pr/1041

Pr/1041

Author: lgebhardt

lib/jsonapi/active_record_accessor.rb

@@ -211,28 +211,33 @@ def apply_pagination(records, paginator, order_options)
 
     def apply_sort(records, order_options, context = {})
       if defined?(_resource_klass.apply_sort)
-        _resource_klass.apply_sort(records, order_options, context)
+        custom_sort = _resource_klass.apply_sort(records, order_options, context)
+        custom_sort.nil? ? default_sort(records, order_options) : custom_sort
       else
-        if order_options.any?
-          order_options.each_pair do |field, direction|
-            if field.to_s.include?(".")
-              *model_names, column_name = field.split(".")
-
-              associations = _lookup_association_chain([records.model.to_s, *model_names])
-              joins_query = _build_joins([records.model, *associations])
-
-              # _sorting is appended to avoid name clashes with manual joins eg. overridden filters
-              order_by_query = "#{associations.last.name}_sorting.#{column_name} #{direction}"
-              records = records.joins(joins_query).order(order_by_query)
-            else
-              field = _resource_klass._attribute_delegated_name(field)
-              records = records.order(field => direction)
-            end
+        default_sort(records, order_options)
+      end
+    end
+
+    def default_sort(records, order_options)
+      if order_options.any?
+        order_options.each_pair do |field, direction|
+          if field.to_s.include?(".")
+            *model_names, column_name = field.split(".")
+
+            associations = _lookup_association_chain([records.model.to_s, *model_names])
+            joins_query = _build_joins([records.model, *associations])
+
+            # _sorting is appended to avoid name clashes with manual joins eg. overridden filters
+            order_by_query = "#{associations.last.name}_sorting.#{column_name} #{direction}"
+            records = records.joins(joins_query).order(order_by_query)
+          else
+            field = _resource_klass._attribute_delegated_name(field)
+            records = records.order(field => direction)
           end
         end
-
-        records
       end
+
+      records
     end
 
     def _lookup_association_chain(model_names)
@@ -474,7 +479,7 @@ def preload_included_fragments(src_res_class, resource_pile, path, serializer, o
         next unless src_res
         fragment = target_resources[tgt_id]
         next unless fragment
-        src_res.preloaded_fragments[serialized_rel_name][tgt_id] = fragment 
+        src_res.preloaded_fragments[serialized_rel_name][tgt_id] = fragment
       end
     end
 

test/unit/resource/resource_test.rb

@@ -391,12 +391,36 @@ def apply_filters(records, filters, options)
     end
   end
 
+  def test_custom_sorting
+    post_resource = PostResource.new(Post.find(1), nil)
+    comment_ids = post_resource.comments.map{|c| c._model.id }
+    assert_equal [1,2], comment_ids
+
+    # define apply_sort method on post resource that will never sort
+    PostResource.instance_eval do
+      def apply_sort(records, criteria, context = {})
+        if criteria.key?('name')
+          # this sort will never occure
+          records.order('name asc')
+        end
+      end
+    end
+
+    sorted_comment_ids = post_resource.comments(sort_criteria: [{ field: 'id', direction: :desc}]).map{|c| c._model.id }
+    assert_equal [2,1], sorted_comment_ids
+  ensure
+    # reset method to original implementation
+    PostResource.instance_eval do
+      undef :apply_sort
+    end
+  end
+
   def test_to_many_relationship_sorts
     post_resource = PostResource.new(Post.find(1), nil)
     comment_ids = post_resource.comments.map{|c| c._model.id }
     assert_equal [1,2], comment_ids
 
-    # define apply_filters method on post resource to sort descending
+    # define apply_sort method on post resource to sort descending
     PostResource.instance_eval do
       def apply_sort(records, criteria, context = {})
         # :nocov:
@@ -412,28 +436,7 @@ def apply_sort(records, criteria, context = {})
   ensure
     # reset method to original implementation
     PostResource.instance_eval do
-      def apply_sort(records, order_options, _context = {})
-        # :nocov:
-        if order_options.any?
-          order_options.each_pair do |field, direction|
-            if field.to_s.include?(".")
-              *model_names, column_name = field.split(".")
-
-              associations = _lookup_association_chain([records.model.to_s, *model_names])
-              joins_query = _record_accessor._build_joins([records.model, *associations])
-
-              # _sorting is appended to avoid name clashes with manual joins eg. overriden filters
-              order_by_query = "#{associations.last.name}_sorting.#{column_name} #{direction}"
-              records = records.joins(joins_query).order(order_by_query)
-            else
-              records = records.order(field => direction)
-            end
-          end
-        end
-
-        records
-        # :nocov:
-      end
+      undef :apply_sort
     end
   end