Skip to content

Commit

Permalink
Merge records in apply_join
Browse files Browse the repository at this point in the history
  • Loading branch information
lgebhardt committed Feb 10, 2020
1 parent ce3084e commit 79cd383
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 20 deletions.
5 changes: 5 additions & 0 deletions lib/jsonapi/active_relation_resource.rb
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,11 @@ def apply_join(records:, relationship:, resource_type:, join_type:, options:)
records = records.joins_left(relation_name)
end
end

if relationship.merge_resource_records
records = records.merge(self.records(options))
end

records
end

Expand Down
4 changes: 2 additions & 2 deletions lib/jsonapi/relationship.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ class Relationship
attr_reader :acts_as_set, :foreign_key, :options, :name,
:class_name, :polymorphic, :always_include_optional_linkage_data,
:parent_resource, :eager_load_on_include, :custom_methods,
:inverse_relationship, :allow_include
:inverse_relationship, :allow_include, :merge_resource_records

attr_writer :allow_include

Expand All @@ -22,7 +22,7 @@ def initialize(name, options = {})
ActiveSupport::Deprecation.warn('Use polymorphic_types instead of polymorphic_relations')
@polymorphic_types ||= options[:polymorphic_relations]
end

@merge_resource_records = options.fetch(:merge_resource_records, options[:relation_name].nil?) == true
@always_include_optional_linkage_data = options.fetch(:always_include_optional_linkage_data, false) == true
@eager_load_on_include = options.fetch(:eager_load_on_include, false) == true
@allow_include = options[:allow_include]
Expand Down
28 changes: 10 additions & 18 deletions test/fixtures/active_record.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1413,7 +1413,7 @@ class PostResource < JSONAPI::Resource

has_one :author, class_name: 'Person'
has_one :section
has_many :tags, acts_as_set: true, inverse_relationship: :posts, eager_load_on_include: false
has_many :tags, acts_as_set: true, inverse_relationship: :posts
has_many :comments, acts_as_set: false, inverse_relationship: :post

# Not needed - just for testing
Expand Down Expand Up @@ -1932,16 +1932,7 @@ class AuthorResource < JSONAPI::Resource
model_name 'Person'
attributes :name

has_many :books, inverse_relationship: :authors, relation_name: -> (options) {
book_admin = options[:context][:book_admin] || options[:context][:current_user].try(:book_admin)

if book_admin
:books
else
:not_banned_books
end
}

has_many :books
has_many :book_comments
end

Expand Down Expand Up @@ -1981,6 +1972,9 @@ class BookResource < JSONAPI::Resource
}

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

class << self
def books
Expand All @@ -1992,10 +1986,9 @@ def not_banned_books
end

def records(options = {})
context = options[:context]
current_user = context ? context[:current_user] : nil
current_user = options.dig(:context, :current_user)

records = _model_class.all
records = super
# Hide the banned books from people who are not book admins
unless current_user && current_user.book_admin
records = records.where(not_banned_books)
Expand All @@ -2004,8 +1997,7 @@ def records(options = {})
end

def apply_filter_banned(records, value, options)
context = options[:context]
current_user = context ? context[:current_user] : nil
current_user = options.dig(:context, :current_user)

# Only book admins might filter for banned books
if current_user && current_user.book_admin
Expand Down Expand Up @@ -2045,7 +2037,7 @@ def approved_comments(approved = true)
end

def records(options = {})
current_user = options[:context][:current_user]
current_user = options.dig(:context, :current_user)
_model_class.for_user(current_user)
end
end
Expand Down Expand Up @@ -2100,7 +2092,7 @@ class PostResource < JSONAPI::Resource

has_one :author, class_name: 'Person', exclude_links: [:self, "related"]
has_one :section, exclude_links: [:self, :related]
has_many :tags, acts_as_set: true, inverse_relationship: :posts, eager_load_on_include: false, exclude_links: :default
has_many :tags, acts_as_set: true, inverse_relationship: :posts, exclude_links: :default
has_many :comments, acts_as_set: false, inverse_relationship: :post, exclude_links: ["self", :related]
end

Expand Down
38 changes: 38 additions & 0 deletions test/integration/book_authorization_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
require File.expand_path('../../test_helper', __FILE__)

class BookAuthorizationTest < ActionDispatch::IntegrationTest
def setup
DatabaseCleaner.start
JSONAPI.configuration.json_key_format = :underscored_key
JSONAPI.configuration.route_format = :underscored_route
Api::V2::BookResource.paginator :offset
end

def test_restricted_records_primary
Api::V2::BookResource.paginator :none

# Not a book admin
$test_user = Person.find(1001)
assert_cacheable_jsonapi_get '/api/v2/books?filter[title]=Book%206'
assert_equal 12, json_response['data'].size

# book_admin
$test_user = Person.find(1005)
assert_cacheable_jsonapi_get '/api/v2/books?filter[title]=Book%206'
assert_equal 111, json_response['data'].size
end

def test_restricted_records_related
Api::V2::BookResource.paginator :none

# Not a book admin
$test_user = Person.find(1001)
assert_cacheable_jsonapi_get '/api/v2/authors/1002/books'
assert_equal 1, json_response['data'].size

# book_admin
$test_user = Person.find(1005)
assert_cacheable_jsonapi_get '/api/v2/authors/1002/books'
assert_equal 2, json_response['data'].size
end
end
24 changes: 24 additions & 0 deletions test/unit/resource/relationship_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,30 @@ def self.is_admin(context)
end
end

class TestRelationshipOptionsPostsResource < JSONAPI::Resource
model_name 'Post'
has_one :author, allow_include: :is_admin, merge_resource_records: false
end

class RelationshipTest < ActiveSupport::TestCase
def test_merge_resource_records_enabled_by_default
relationship = JSONAPI::Relationship::ToOne.new(:author)
assert relationship.merge_resource_records
end

def test_merge_resource_records_is_disabled_by_deafult_with_relation_name
relationship = JSONAPI::Relationship::ToOne.new(:author,
relation_name: "foo" )
refute relationship.merge_resource_records
end

def test_merge_resource_records_can_be_disabled
relationship = JSONAPI::Relationship::ToOne.new(:author,
merge_resource_records: false )
refute relationship.merge_resource_records
end
end

class HasOneRelationshipTest < ActiveSupport::TestCase

def test_polymorphic_type
Expand Down

0 comments on commit 79cd383

Please sign in to comment.