Merge pull request #938 from cerebris/fix_933_master

Fix 933 - Sort with has one include

Author: lgebhardt

lib/jsonapi/resource.rb

@@ -1102,7 +1102,6 @@ def preload_included_fragments(resources, records, serializer, options)
         include_directives = options[:include_directives]
         return unless include_directives
 
-        relevant_options = options.except(:include_directives, :order, :paginator)
         context = options[:context]
 
         # For each association, including indirect associations, find the target record ids.
@@ -1196,7 +1195,7 @@ def preload_included_fragments(resources, records, serializer, options)
               .map(&:last)
               .reject{|id| target_resources[klass.name].has_key?(id) }
               .uniq
-            found = klass.find({klass._primary_key => sub_res_ids}, relevant_options)
+            found = klass.find({klass._primary_key => sub_res_ids}, context: options[:context])
             target_resources[klass.name].merge! found.map{|r| [r.id, r] }.to_h
           end
 

lib/jsonapi/resource_serializer.rb

@@ -50,15 +50,35 @@ def serialize_to_hash(source)
 
       @included_objects = {}
 
-      process_primary(source, @include_directives.include_directives)
+      process_source_objects(source, @include_directives.include_directives)
 
-      included_objects = []
       primary_objects = []
+
+      # pull the processed objects corresponding to the source objects. Ensures we preserve order.
+      if is_resource_collection
+        source.each do |primary|
+          if primary.id
+            case primary
+              when CachedResourceFragment then primary_objects.push(@included_objects[primary.type][primary.id][:object_hash])
+              when Resource then primary_objects.push(@included_objects[primary.class._type][primary.id][:object_hash])
+              else raise "Unknown source type #{primary.inspect}"
+            end
+          end
+        end
+      else
+        if source.try(:id)
+          case source
+            when CachedResourceFragment then primary_objects.push(@included_objects[source.type][source.id][:object_hash])
+            when Resource then primary_objects.push(@included_objects[source.class._type][source.id][:object_hash])
+            else raise "Unknown source type #{source.inspect}"
+          end
+        end
+      end
+
+      included_objects = []
       @included_objects.each_value do |objects|
         objects.each_value do |object|
-          if object[:primary]
-            primary_objects.push(object[:object_hash])
-          else
+          unless object[:primary]
             included_objects.push(object[:object_hash])
           end
         end
@@ -168,9 +188,9 @@ def object_hash(source, include_directives = {})
     # requested includes. Fields are controlled fields option for each resource type, such
     # as fields: { people: [:id, :email, :comments], posts: [:id, :title, :author], comments: [:id, :body, :post]}
     # The fields options controls both fields and included links references.
-    def process_primary(source, include_directives)
+    def process_source_objects(source, include_directives)
       if source.respond_to?(:to_ary)
-        source.each { |resource| process_primary(resource, include_directives) }
+        source.each { |resource| process_source_objects(resource, include_directives) }
       else
         return {} if source.nil?
         add_resource(source, include_directives, true)

test/controllers/controller_test.rb

@@ -423,7 +423,7 @@ def test_sorting_by_relationship_field
     assert_cacheable_get :index, params: {sort: 'author.name'}
 
     assert_response :success
-    assert json_response['data'].length > 10, 'there are enough recordsto show sort'
+    assert json_response['data'].length > 10, 'there are enough records to show sort'
     assert_equal '17', json_response['data'][0]['id'], 'nil is at the top'
     assert_equal post.id.to_s, json_response['data'][1]['id'], 'alphabetically first user is second'
   end
@@ -438,6 +438,16 @@ def test_desc_sorting_by_relationship_field
     assert_equal post.id.to_s, json_response['data'][-2]['id'], 'alphabetically first user is second last'
   end
 
+  def test_sorting_by_relationship_field_include
+    post  = create_alphabetically_first_user_and_post
+    assert_cacheable_get :index, params: {include: 'author', sort: 'author.name'}
+
+    assert_response :success
+    assert json_response['data'].length > 10, 'there are enough records to show sort'
+    assert_equal '17', json_response['data'][0]['id'], 'nil is at the top'
+    assert_equal post.id.to_s, json_response['data'][1]['id'], 'alphabetically first user is second'
+  end
+
   def test_invalid_sort_param
     assert_cacheable_get :index, params: {sort: 'asdfg'}
 
@@ -1921,6 +1931,15 @@ def test_tags_show_multiple_with_nonexistent_ids_at_the_beginning
     assert_response :bad_request
     assert_match /99,9,100 is not a valid value for id/, response.body
   end
+
+  def test_nested_includes_sort
+    assert_cacheable_get :index, params: {filter: {id: '6,7,8,9'},
+                                          include: 'posts.tags,posts.author.posts',
+                                          sort: 'name'}
+    assert_response :success
+    assert_equal 4, json_response['data'].size
+    assert_equal 3, json_response['included'].size
+  end
 end
 
 class PicturesControllerTest < ActionController::TestCase

test/fixtures/active_record.rb

@@ -1138,10 +1138,6 @@ class PlanetResource < JSONAPI::Resource
   has_one :planet_type
 
   has_many :tags, acts_as_set: true
-
-  def records_for_moons(opts = {})
-    Moon.joins(:craters).select('moons.*, craters.code').distinct
-  end
 end
 
 class PropertyResource < JSONAPI::Resource