Merge pull request #494 from kinesisptyltd/master

[#490] Add support for sorting relationship fields.

Author: lgebhardt

README.md

@@ -287,6 +287,32 @@ class PostResource < JSONAPI::Resource
 end
 ```
 
+JR also supports sorting primary resources by fields on relationships.
+
+Here's an example of sorting books by the author name:
+
+```ruby
+class Book < ActiveRecord::Base
+    belongs_to :author
+end
+
+class Author < ActiveRecord::Base
+    has_many :books
+end
+
+class BookResource < JSONAPI::Resource
+  attributes :title, :body
+
+  def self.sortable_fields(context)
+    super(context) << :"author.name"
+   end
+end
+```
+The request will look something like:
+```
+GET /books?include=author&sort=author.name
+```
+
 ##### Attribute Formatting
 
 Attributes can have a `Format`. By default all attributes use the default formatter. If an attribute has the `format`

lib/jsonapi/resource.rb

@@ -530,10 +530,46 @@ def apply_pagination(records, paginator, order_options)
 
       def apply_sort(records, order_options, _context = {})
         if order_options.any?
-          records.order(order_options)
-        else
-          records
+           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. 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
+      end
+
+      def _lookup_association_chain(model_names)
+        associations = []
+        model_names.inject do |prev, current|
+          association = prev.classify.constantize.reflect_on_all_associations.detect do |assoc|
+            assoc.name.to_s.downcase == current.downcase
+          end
+          associations << association
+          association.class_name
+        end
+
+        associations
+      end
+
+      def _build_joins(associations)
+        joins = []
+
+        associations.inject do |prev, current|
+          joins << "LEFT JOIN #{current.table_name} AS #{current.name}_sorting ON #{current.name}_sorting.id = #{prev.table_name}.#{current.foreign_key}"
+          current
         end
+        joins.join("\n")
       end
 
       def apply_filter(records, filter, value, options = {})

lib/jsonapi/resource_serializer.rb

@@ -31,6 +31,8 @@ def initialize(primary_resource_klass, options = {})
 
     # Converts a single resource, or an array of resources to a hash, conforming to the JSONAPI structure
     def serialize_to_hash(source)
+      @top_level_sources = Set.new([source].flatten.compact.map {|s| top_level_source_key(s) })
+
       is_resource_collection = source.respond_to?(:to_ary)
 
       @included_objects = {}
@@ -174,6 +176,14 @@ def custom_links_hash(source)
       (custom_links.is_a?(Hash) && custom_links) || {}
     end
 
+    def top_level_source_key(source)
+      "#{source.class}_#{source.id}"
+    end
+
+    def self_referential_and_already_in_source(resource)
+      resource && @top_level_sources.include?(top_level_source_key(resource))
+    end
+
     def relationships_hash(source, include_directives)
       relationships = source.class._relationships
       requested = requested_fields(source.class)
@@ -192,39 +202,24 @@ def relationships_hash(source, include_directives)
 
           include_linkage = ia && ia[:include]
           include_linked_children = ia && !ia[:include_related].empty?
+          resources = (include_linkage || include_linked_children) && [source.public_send(name)].flatten.compact
 
           if field_set.include?(name)
             hash[format_key(name)] = link_object(source, relationship, include_linkage)
           end
 
-          type = relationship.type
-
           # If the object has been serialized once it will be in the related objects list,
           # but it's possible all children won't have been captured. So we must still go
           # through the relationships.
           if include_linkage || include_linked_children
-            if relationship.is_a?(JSONAPI::Relationship::ToOne)
-              resource = source.public_send(name)
-              if resource
-                id = resource.id
-                type = relationship.type_for_source(source)
-                relationships_only = already_serialized?(type, id)
-                if include_linkage && !relationships_only
-                  add_included_object(id, object_hash(resource, ia))
-                elsif include_linked_children || relationships_only
-                  relationships_hash(resource, ia)
-                end
-              end
-            elsif relationship.is_a?(JSONAPI::Relationship::ToMany)
-              resources = source.public_send(name)
-              resources.each do |resource|
-                id = resource.id
-                relationships_only = already_serialized?(type, id)
-                if include_linkage && !relationships_only
-                  add_included_object(id, object_hash(resource, ia))
-                elsif include_linked_children || relationships_only
-                  relationships_hash(resource, ia)
-                end
+            resources.each do |resource|
+              next if self_referential_and_already_in_source(resource)
+              id = resource.id
+              relationships_only = already_serialized?(relationship.type, id)
+              if include_linkage && !relationships_only
+                add_included_object(id, object_hash(resource, ia))
+              elsif include_linked_children || relationships_only
+                relationships_hash(resource, ia)
               end
             end
           end

test/controllers/controller_test.rb

@@ -302,6 +302,31 @@ def test_sorting_by_multiple_fields
     assert_equal '14', json_response['data'][0]['id']
   end
 
+  def create_alphabetically_first_user_and_post
+    author = Person.create(name: "Aardvark", date_joined: Time.now)
+    author.posts.create(title: "My first post", body: "Hello World")
+  end
+
+  def test_sorting_by_relationship_field
+    post  = create_alphabetically_first_user_and_post
+    get :index, params: {sort: 'author.name'}
+
+    assert_response :success
+    assert json_response['data'].length > 10, 'there are enough recordsto 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_desc_sorting_by_relationship_field
+    post  = create_alphabetically_first_user_and_post
+    get :index, {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'][-1]['id'], 'nil is at the bottom'
+    assert_equal post.id.to_s, json_response['data'][-2]['id'], 'alphabetically first user is second last'
+  end
+
   def test_invalid_sort_param
     get :index, params: {sort: 'asdfg'}
 

test/fixtures/active_record.rb

@@ -28,6 +28,7 @@
     t.string     :title
     t.text       :body
     t.integer    :author_id
+    t.integer    :parent_post_id
     t.belongs_to :section, index: true
     t.timestamps null: false
   end
@@ -279,6 +280,7 @@ class Post < ActiveRecord::Base
   has_many :special_post_tags, source: :tag
   has_many :special_tags, through: :special_post_tags, source: :tag
   belongs_to :section
+  has_one :parent_post, class_name: 'Post', foreign_key: 'parent_post_id'
 
   validates :author, presence: true
   validates :title, length: { maximum: 35 }
@@ -885,6 +887,14 @@ class SectionResource < JSONAPI::Resource
   attributes 'name'
 end
 
+module ParentApi
+  class PostResource < JSONAPI::Resource
+    model_name 'Post'
+    attributes :title
+    has_one :parent_post
+  end
+end
+
 class PostResource < JSONAPI::Resource
   attribute :title
   attribute :body
@@ -895,7 +905,6 @@ class PostResource < JSONAPI::Resource
   has_many :tags, acts_as_set: true
   has_many :comments, acts_as_set: false
 
-
   # Not needed - just for testing
   primary_key :id
 
@@ -973,9 +982,9 @@ def self.creatable_fields(context)
   end
 
   def self.sortable_fields(context)
-    super(context) - [:id]
+    super(context) - [:id] + [:"author.name"]
   end
-
+ 
   def self.verify_key(key, context = nil)
     super(key)
     raise JSONAPI::Exceptions::RecordNotFound.new(key) unless find_by_key(key, context: context)
@@ -1227,7 +1236,6 @@ def custom_links(options)
   end
 end
 
-
 module Api
   module V1
     class WriterResource < JSONAPI::Resource

test/unit/jsonapi_request/jsonapi_request_test.rb

@@ -8,6 +8,10 @@ class CatResource < JSONAPI::Resource
   has_one :father, class_name: 'Cat'
 
   filters :name
+
+  def self.sortable_fields(context)
+    super(context) << :"mother.name"
+  end
 end
 
 class JSONAPIRequestTest < ActiveSupport::TestCase
@@ -190,6 +194,22 @@ def test_parse_filters_with_invalid_filters_param
     assert_equal(@request.errors.first.title, "Invalid filters syntax")
   end
 
+  def test_parse_sort_with_valid_sorts
+    setup_request
+    @request.parse_sort_criteria("-name")
+    assert_equal(@request.filters, {})
+    assert_equal(@request.errors, [])
+    assert_equal(@request.sort_criteria, [{:field=>"name", :direction=>:desc}])
+  end
+
+  def test_parse_sort_with_relationships
+    setup_request
+    @request.parse_sort_criteria("-mother.name")
+    assert_equal(@request.filters, {})
+    assert_equal(@request.errors, [])
+    assert_equal(@request.sort_criteria, [{:field=>"mother.name", :direction=>:desc}])
+  end
+
   private
 
   def setup_request

test/unit/resource/resource_test.rb

@@ -342,6 +342,31 @@ def apply_sort(records, criteria, context = {})
     end
   end
 
+  def test_lookup_association_chain
+    model_names = %w(person posts parent_post)
+    result = PostResource._lookup_association_chain(model_names)
+    assert_equal 2, result.length
+
+    posts_reflection, parent_post_reflection = result
+    assert_equal :posts, posts_reflection.name
+    assert_equal :parent_post, parent_post_reflection.name
+
+    assert_equal "posts", posts_reflection.table_name
+    assert_equal "posts", parent_post_reflection.table_name
+
+    assert_equal "author_id", posts_reflection.foreign_key
+    assert_equal "parent_post_id", parent_post_reflection.foreign_key
+  end
+
+  def test_build_joins
+    model_names = %w(person posts parent_post author)
+    associations = PostResource._lookup_association_chain(model_names)
+    result = PostResource._build_joins(associations)
+
+    assert_equal "LEFT JOIN posts AS parent_post_sorting ON parent_post_sorting.id = posts.parent_post_id
+LEFT JOIN people AS author_sorting ON author_sorting.id = posts.author_id", result
+  end
+
   def test_to_many_relationship_pagination
     post_resource = PostResource.new(Post.find(1), nil)
     comments = post_resource.comments

test/unit/serializer/serializer_test.rb

@@ -529,6 +529,23 @@ def test_serializer_include_sub_objects
     )
   end
 
+  def test_serializer_keeps_sorted_order_of_objects_with_self_referential_relationships
+    post1, post2, post3 = Post.find(1), Post.find(2), Post.find(3)
+    post1.parent_post = post3
+    ordered_posts = [post1, post2, post3]
+    serialized_data = JSONAPI::ResourceSerializer.new(
+      ParentApi::PostResource,
+      include: ['parent_post'],
+      base_url: 'http://example.com').serialize_to_hash(ordered_posts.map {|p| ParentApi::PostResource.new(p, nil)} 
+    )[:data]
+
+    assert_equal(3, serialized_data.length)
+    assert_equal("1", serialized_data[0]["id"])
+    assert_equal("2", serialized_data[1]["id"])
+    assert_equal("3", serialized_data[2]["id"])
+  end
+
+ 
   def test_serializer_different_foreign_key
     serialized = JSONAPI::ResourceSerializer.new(
       PersonResource,