Skip to content

Commit 346d001

Browse files
committed
Fix multi joins in resource and refactor methods to be pseudo private.
1 parent d0b0d7c commit 346d001

3 files changed

Lines changed: 50 additions & 15 deletions

File tree

lib/jsonapi/resource.rb

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -534,8 +534,8 @@ def apply_sort(records, order_options, _context = {})
534534
if field.to_s.include?(".")
535535
*model_names, column_name = field.split(".")
536536

537-
associations = lookup_association_chain([records.model.to_s, *model_names])
538-
joins_query = build_joins([records.model, *associations])
537+
associations = _lookup_association_chain([records.model.to_s, *model_names])
538+
joins_query = _build_joins([records.model, *associations])
539539

540540
order_by_query = "#{associations.last.name}_sorting.#{column_name} #{direction}"
541541
records = records.joins(joins_query).order(order_by_query)
@@ -548,28 +548,27 @@ def apply_sort(records, order_options, _context = {})
548548
records
549549
end
550550

551-
def lookup_association_chain(model_names)
551+
def _lookup_association_chain(model_names)
552552
associations = []
553553
model_names.inject do |prev, current|
554-
p = prev.classify.constantize.reflect_on_all_associations.detect do |assoc|
554+
association = prev.classify.constantize.reflect_on_all_associations.detect do |assoc|
555555
assoc.name.to_s.downcase == current.downcase
556556
end
557-
associations << p
558-
p.class_name
557+
associations << association
558+
association.class_name
559559
end
560560

561561
associations
562562
end
563563

564-
def build_joins(associations)
565-
joins = ""
564+
def _build_joins(associations)
565+
joins = []
566566

567567
associations.inject do |prev, current|
568568
joins << "LEFT JOIN #{current.table_name} AS #{current.name}_sorting ON #{current.name}_sorting.id = #{prev.table_name}.#{current.foreign_key}"
569569
current
570570
end
571-
572-
joins
571+
joins.join("\n")
573572
end
574573

575574
def apply_filter(records, filter, value, options = {})

test/controllers/controller_test.rb

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -302,20 +302,31 @@ def test_sorting_by_multiple_fields
302302
assert_equal '14', json_response['data'][0]['id']
303303
end
304304

305+
def create_alphabetically_first_user_and_post
306+
author = Person.create(name: "Aardvark", date_joined: Time.now)
307+
author.posts.create(title: "My first post", body: "Hello World")
308+
end
309+
305310
def test_sorting_by_relationship_field
306-
get :index, {sort: 'author.name'}
311+
post = create_alphabetically_first_user_and_post
312+
get :index, params: {sort: 'author.name'}
307313

308314
assert_response :success
309-
assert json_response['data'].length > 10
310-
assert_equal '17', json_response['data'][0]['id']
315+
assert json_response['data'].length > 10, 'there are enough recordsto show sort'
316+
assert_equal '17', json_response['data'][0]['id'], 'nil is at the top'
317+
assert_equal post.id.to_s, json_response['data'][1]['id'], 'alphabetically first user is second'
311318
end
312319

313320
def test_desc_sorting_by_relationship_field
321+
post = create_alphabetically_first_user_and_post
314322
get :index, {sort: '-author.name'}
315323

316324
assert_response :success
317-
assert json_response['data'].length > 10
318-
assert_equal '17', json_response['data'][-1]['id']
325+
assert json_response['data'].length > 10, 'there are enough records to show sort'
326+
assert_equal '17', json_response['data'][-1]['id'], 'nil is at the bottom'
327+
assert_equal post.id.to_s, json_response['data'][-2]['id'], 'alphabetically first user is second last'
328+
end
329+
319330
end
320331

321332
def test_invalid_sort_param

test/unit/resource/resource_test.rb

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -342,6 +342,31 @@ def apply_sort(records, criteria, context = {})
342342
end
343343
end
344344

345+
def test_lookup_association_chain
346+
model_names = %w(person posts parent_post)
347+
result = PostResource._lookup_association_chain(model_names)
348+
assert_equal 2, result.length
349+
350+
posts_reflection, parent_post_reflection = result
351+
assert_equal :posts, posts_reflection.name
352+
assert_equal :parent_post, parent_post_reflection.name
353+
354+
assert_equal "posts", posts_reflection.table_name
355+
assert_equal "posts", parent_post_reflection.table_name
356+
357+
assert_equal "author_id", posts_reflection.foreign_key
358+
assert_equal "parent_post_id", parent_post_reflection.foreign_key
359+
end
360+
361+
def test_build_joins
362+
model_names = %w(person posts parent_post author)
363+
associations = PostResource._lookup_association_chain(model_names)
364+
result = PostResource._build_joins(associations)
365+
366+
assert_equal "LEFT JOIN posts AS parent_post_sorting ON parent_post_sorting.id = posts.parent_post_id
367+
LEFT JOIN people AS author_sorting ON author_sorting.id = posts.author_id", result
368+
end
369+
345370
def test_to_many_relationship_pagination
346371
post_resource = PostResource.new(Post.find(1), nil)
347372
comments = post_resource.comments

0 commit comments

Comments
 (0)