From 79cd38311eb78f91a679d3d2d40098315e259657 Mon Sep 17 00:00:00 2001 From: Larry Gebhardt Date: Sat, 8 Feb 2020 11:44:50 -0500 Subject: [PATCH] Merge `records` in `apply_join` --- lib/jsonapi/active_relation_resource.rb | 5 +++ lib/jsonapi/relationship.rb | 4 +-- test/fixtures/active_record.rb | 28 ++++++--------- test/integration/book_authorization_test.rb | 38 +++++++++++++++++++++ test/unit/resource/relationship_test.rb | 24 +++++++++++++ 5 files changed, 79 insertions(+), 20 deletions(-) create mode 100644 test/integration/book_authorization_test.rb diff --git a/lib/jsonapi/active_relation_resource.rb b/lib/jsonapi/active_relation_resource.rb index d9272339d..62f6f38a5 100644 --- a/lib/jsonapi/active_relation_resource.rb +++ b/lib/jsonapi/active_relation_resource.rb @@ -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 diff --git a/lib/jsonapi/relationship.rb b/lib/jsonapi/relationship.rb index 77e700b78..210e0a747 100644 --- a/lib/jsonapi/relationship.rb +++ b/lib/jsonapi/relationship.rb @@ -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 @@ -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] diff --git a/test/fixtures/active_record.rb b/test/fixtures/active_record.rb index bdb718bbf..ee2ac77de 100644 --- a/test/fixtures/active_record.rb +++ b/test/fixtures/active_record.rb @@ -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 @@ -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 @@ -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 @@ -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) @@ -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 @@ -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 @@ -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 diff --git a/test/integration/book_authorization_test.rb b/test/integration/book_authorization_test.rb new file mode 100644 index 000000000..6327f2ba3 --- /dev/null +++ b/test/integration/book_authorization_test.rb @@ -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 diff --git a/test/unit/resource/relationship_test.rb b/test/unit/resource/relationship_test.rb index a98d26601..ae28b4b36 100644 --- a/test/unit/resource/relationship_test.rb +++ b/test/unit/resource/relationship_test.rb @@ -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