Skip to content

Commit 58bdf36

Browse files
committed
Merge records in apply_join
1 parent 9d30ea3 commit 58bdf36

File tree

6 files changed

+85
-26
lines changed

6 files changed

+85
-26
lines changed

lib/jsonapi/active_relation_resource.rb

+5
Original file line numberDiff line numberDiff line change
@@ -338,6 +338,11 @@ def apply_join(records:, relationship:, resource_type:, join_type:, options:)
338338
records = records.joins_left(relation_name)
339339
end
340340
end
341+
342+
if relationship.merge_resource_records
343+
records = records.merge(self.records(options))
344+
end
345+
341346
records
342347
end
343348

lib/jsonapi/relationship.rb

+2-2
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ class Relationship
33
attr_reader :acts_as_set, :foreign_key, :options, :name,
44
:class_name, :polymorphic, :always_include_optional_linkage_data,
55
:parent_resource, :eager_load_on_include, :custom_methods,
6-
:inverse_relationship, :allow_include
6+
:inverse_relationship, :allow_include, :merge_resource_records
77

88
attr_writer :allow_include
99

@@ -22,7 +22,7 @@ def initialize(name, options = {})
2222
ActiveSupport::Deprecation.warn('Use polymorphic_types instead of polymorphic_relations')
2323
@polymorphic_types ||= options[:polymorphic_relations]
2424
end
25-
25+
@merge_resource_records = options.fetch(:merge_resource_records, options[:relation_name].nil?) == true
2626
@always_include_optional_linkage_data = options.fetch(:always_include_optional_linkage_data, false) == true
2727
@eager_load_on_include = options.fetch(:eager_load_on_include, true) == true
2828
@allow_include = options[:allow_include]

test/fixtures/active_record.rb

+10-18
Original file line numberDiff line numberDiff line change
@@ -1413,7 +1413,7 @@ class PostResource < JSONAPI::Resource
14131413

14141414
has_one :author, class_name: 'Person'
14151415
has_one :section
1416-
has_many :tags, acts_as_set: true, inverse_relationship: :posts, eager_load_on_include: false
1416+
has_many :tags, acts_as_set: true, inverse_relationship: :posts
14171417
has_many :comments, acts_as_set: false, inverse_relationship: :post
14181418

14191419
# Not needed - just for testing
@@ -1937,16 +1937,7 @@ class AuthorResource < JSONAPI::Resource
19371937
model_name 'Person'
19381938
attributes :name
19391939

1940-
has_many :books, inverse_relationship: :authors, relation_name: -> (options) {
1941-
book_admin = options[:context][:book_admin] || options[:context][:current_user].try(:book_admin)
1942-
1943-
if book_admin
1944-
:books
1945-
else
1946-
:not_banned_books
1947-
end
1948-
}
1949-
1940+
has_many :books
19501941
has_many :book_comments
19511942
end
19521943

@@ -1986,6 +1977,9 @@ class BookResource < JSONAPI::Resource
19861977
}
19871978

19881979
filter :banned, apply: :apply_filter_banned
1980+
filter :title, apply: ->(records, value, options) {
1981+
records.where('books.title LIKE ?', "#{value[0]}%")
1982+
}
19891983

19901984
class << self
19911985
def books
@@ -1997,10 +1991,9 @@ def not_banned_books
19971991
end
19981992

19991993
def records(options = {})
2000-
context = options[:context]
2001-
current_user = context ? context[:current_user] : nil
1994+
current_user = options.dig(:context, :current_user)
20021995

2003-
records = _model_class.all
1996+
records = super
20041997
# Hide the banned books from people who are not book admins
20051998
unless current_user && current_user.book_admin
20061999
records = records.where(not_banned_books)
@@ -2009,8 +2002,7 @@ def records(options = {})
20092002
end
20102003

20112004
def apply_filter_banned(records, value, options)
2012-
context = options[:context]
2013-
current_user = context ? context[:current_user] : nil
2005+
current_user = options.dig(:context, :current_user)
20142006

20152007
# Only book admins might filter for banned books
20162008
if current_user && current_user.book_admin
@@ -2050,7 +2042,7 @@ def approved_comments(approved = true)
20502042
end
20512043

20522044
def records(options = {})
2053-
current_user = options[:context][:current_user]
2045+
current_user = options.dig(:context, :current_user)
20542046
_model_class.for_user(current_user)
20552047
end
20562048
end
@@ -2105,7 +2097,7 @@ class PostResource < JSONAPI::Resource
21052097

21062098
has_one :author, class_name: 'Person', exclude_links: [:self, "related"]
21072099
has_one :section, exclude_links: [:self, :related]
2108-
has_many :tags, acts_as_set: true, inverse_relationship: :posts, eager_load_on_include: false, exclude_links: :default
2100+
has_many :tags, acts_as_set: true, inverse_relationship: :posts, exclude_links: :default
21092101
has_many :comments, acts_as_set: false, inverse_relationship: :post, exclude_links: ["self", :related]
21102102
end
21112103

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
require File.expand_path('../../test_helper', __FILE__)
2+
3+
class BookAuthorizationTest < ActionDispatch::IntegrationTest
4+
def setup
5+
DatabaseCleaner.start
6+
JSONAPI.configuration.json_key_format = :underscored_key
7+
JSONAPI.configuration.route_format = :underscored_route
8+
Api::V2::BookResource.paginator :offset
9+
end
10+
11+
def test_restricted_records_primary
12+
Api::V2::BookResource.paginator :none
13+
14+
# Not a book admin
15+
$test_user = Person.find(1001)
16+
assert_cacheable_jsonapi_get '/api/v2/books?filter[title]=Book%206'
17+
assert_equal 12, json_response['data'].size
18+
19+
# book_admin
20+
$test_user = Person.find(1005)
21+
assert_cacheable_jsonapi_get '/api/v2/books?filter[title]=Book%206'
22+
assert_equal 111, json_response['data'].size
23+
end
24+
25+
def test_restricted_records_related
26+
Api::V2::BookResource.paginator :none
27+
28+
# Not a book admin
29+
$test_user = Person.find(1001)
30+
assert_cacheable_jsonapi_get '/api/v2/authors/1002/books'
31+
assert_equal 1, json_response['data'].size
32+
33+
# book_admin
34+
$test_user = Person.find(1005)
35+
assert_cacheable_jsonapi_get '/api/v2/authors/1002/books'
36+
assert_equal 2, json_response['data'].size
37+
end
38+
end

test/unit/active_relation_resource_finder/join_manager_test.rb

+6-6
Original file line numberDiff line numberDiff line change
@@ -218,18 +218,18 @@ def test_add_joins_with_sub_relationship
218218

219219
if (Rails::VERSION::MAJOR == 6 && Rails::VERSION::MINOR >= 1) || Rails::VERSION::MAJOR > 6
220220
sql = 'SELECT "posts".* FROM "posts" INNER JOIN "comments" ON "comments"."post_id" = "posts"."id" LEFT OUTER JOIN "people" author ON author."id" = "comments"."author_id" LEFT OUTER JOIN "comments_tags" ON "comments_tags"."comment_id" = "comments"."id" LEFT OUTER JOIN "tags" ON "tags"."id" = "comments_tags"."tag_id" LEFT OUTER JOIN "comments" "comments_people" ON "comments_people"."author_id" = "people"."id" WHERE "comments"."approved" = ' + db_true + ' AND "author"."special" = ' + db_true
221-
assert_hash_equals({alias: 'author', join_type: :left}, join_manager.join_details_by_relationship(Api::V10::CommentResource._relationship(:author)))
221+
assert_hash_equals({alias: 'author', join_type: :left}, join_manager.join_details_by_relationship(Api::V10::CommentResource._relationship(:author)).except!(:join_options))
222222
else
223223
sql = 'SELECT "posts".* FROM "posts" INNER JOIN "comments" ON "comments"."post_id" = "posts"."id" LEFT OUTER JOIN "people" ON "people"."id" = "comments"."author_id" LEFT OUTER JOIN "comments_tags" ON "comments_tags"."comment_id" = "comments"."id" LEFT OUTER JOIN "tags" ON "tags"."id" = "comments_tags"."tag_id" LEFT OUTER JOIN "comments" "comments_people" ON "comments_people"."author_id" = "people"."id" WHERE "comments"."approved" = ' + db_true + ' AND "author"."special" = ' + db_true
224-
assert_hash_equals({alias: 'people', join_type: :left}, join_manager.join_details_by_relationship(Api::V10::CommentResource._relationship(:author)))
224+
assert_hash_equals({alias: 'people', join_type: :left}, join_manager.join_details_by_relationship(Api::V10::CommentResource._relationship(:author)).except!(:join_options))
225225
end
226226

227227
assert_equal sql, records.to_sql
228228

229-
assert_hash_equals({alias: 'comments', join_type: :inner}, join_manager.source_join_details)
230-
assert_hash_equals({alias: 'comments', join_type: :inner}, join_manager.join_details_by_relationship(Api::V10::PostResource._relationship(:comments)))
231-
assert_hash_equals({alias: 'tags', join_type: :left}, join_manager.join_details_by_relationship(Api::V10::CommentResource._relationship(:tags)))
232-
assert_hash_equals({alias: 'comments_people', join_type: :left}, join_manager.join_details_by_relationship(Api::V10::PersonResource._relationship(:comments)))
229+
assert_hash_equals({alias: 'comments', join_type: :inner}, join_manager.source_join_details.except!(:join_options))
230+
assert_hash_equals({alias: 'comments', join_type: :inner}, join_manager.join_details_by_relationship(Api::V10::PostResource._relationship(:comments)).except!(:join_options))
231+
assert_hash_equals({alias: 'tags', join_type: :left}, join_manager.join_details_by_relationship(Api::V10::CommentResource._relationship(:tags)).except!(:join_options))
232+
assert_hash_equals({alias: 'comments_people', join_type: :left}, join_manager.join_details_by_relationship(Api::V10::PersonResource._relationship(:comments)).except!(:join_options))
233233
end
234234

235235
def test_add_joins_with_sub_relationship_and_filters

test/unit/resource/relationship_test.rb

+24
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,30 @@ def self.is_admin(context)
1818
end
1919
end
2020

21+
class TestRelationshipOptionsPostsResource < JSONAPI::Resource
22+
model_name 'Post'
23+
has_one :author, allow_include: :is_admin, merge_resource_records: false
24+
end
25+
26+
class RelationshipTest < ActiveSupport::TestCase
27+
def test_merge_resource_records_enabled_by_default
28+
relationship = JSONAPI::Relationship::ToOne.new(:author)
29+
assert relationship.merge_resource_records
30+
end
31+
32+
def test_merge_resource_records_is_disabled_by_deafult_with_relation_name
33+
relationship = JSONAPI::Relationship::ToOne.new(:author,
34+
relation_name: "foo" )
35+
refute relationship.merge_resource_records
36+
end
37+
38+
def test_merge_resource_records_can_be_disabled
39+
relationship = JSONAPI::Relationship::ToOne.new(:author,
40+
merge_resource_records: false )
41+
refute relationship.merge_resource_records
42+
end
43+
end
44+
2145
class HasOneRelationshipTest < ActiveSupport::TestCase
2246

2347
def test_polymorphic_type

0 commit comments

Comments
 (0)