From b1c980ac53845666045e9b161e6ae39d9593393e Mon Sep 17 00:00:00 2001 From: Carl Thuringer Date: Wed, 20 Mar 2019 15:36:36 -0500 Subject: [PATCH] Pass `rel_opts` when filtering records for record count Naturally I uncovered this because it caused a more complex bug in my app. The problem here is that if you have a filter which depends on something that is set in, for example, the context, you will not have that context when the filter is re-invoked for record count, which happens when the record count configuration is active. This only happens when fetching related resources. In this case I add a test where we try to fetch an author's banned books, of which there should be only one. However, because the prior code discards the rel_opts and instead provides an empty hash (!) the filter does not execute because it will only operate when an admin user is in the context. --- lib/jsonapi/processor.rb | 2 +- test/controllers/controller_test.rb | 39 +++++++++++++++++++++++++++++ test/fixtures/active_record.rb | 2 ++ 3 files changed, 42 insertions(+), 1 deletion(-) diff --git a/lib/jsonapi/processor.rb b/lib/jsonapi/processor.rb index bdd1e13bd..b46601c7a 100644 --- a/lib/jsonapi/processor.rb +++ b/lib/jsonapi/processor.rb @@ -197,7 +197,7 @@ def show_related_resources (paginator && paginator.class.requires_record_count) || (JSONAPI.configuration.top_level_meta_include_page_count)) related_resource_records = source_resource.public_send("records_for_" + relationship_type) - records = resource_klass.filter_records(verified_filters, {}, + records = resource_klass.filter_records(verified_filters, rel_opts, related_resource_records) record_count = resource_klass.count_records(records) diff --git a/test/controllers/controller_test.rb b/test/controllers/controller_test.rb index 6d16ad33d..e5908dd79 100644 --- a/test/controllers/controller_test.rb +++ b/test/controllers/controller_test.rb @@ -2754,6 +2754,45 @@ def test_get_related_resources_with_filters end end +class Api::V2::BooksControllerTest < ActionController::TestCase + def test_get_related_resources_with_filters + $test_user = Person.find(5) + original_config = JSONAPI.configuration.dup + JSONAPI.configuration.top_level_meta_include_record_count = true + JSONAPI.configuration.json_key_format = :dasherized_key + assert_cacheable_get :get_related_resources, + params: { + author_id: '1', + relationship: 'books', + source: 'api/v2/authors', + filter: { fiction: 'true' } + } + assert_response :success + assert_equal 1, json_response['meta']['record-count'] + ensure + JSONAPI.configuration = original_config + end + + def test_get_related_resources_with_filters_2 + # Admin user can find an author's banned books + $test_user = Person.find(5) + original_config = JSONAPI.configuration.dup + JSONAPI.configuration.top_level_meta_include_record_count = true + JSONAPI.configuration.json_key_format = :dasherized_key + assert_cacheable_get :get_related_resources, + params: { + author_id: '1', + relationship: 'books', + source: 'api/v2/authors', + filter: { banned: 'true' } + } + assert_response :success + assert_equal 1, json_response['meta']['record-count'] + ensure + JSONAPI.configuration = original_config + end +end + class BreedsControllerTest < ActionController::TestCase # Note: Breed names go through the TitleValueFormatter diff --git a/test/fixtures/active_record.rb b/test/fixtures/active_record.rb index f2f38dd5e..2a43a97bc 100644 --- a/test/fixtures/active_record.rb +++ b/test/fixtures/active_record.rb @@ -1552,6 +1552,8 @@ def apply_filter_banned(records, value, options) # Only book admins might filter for banned books if current_user && current_user.book_admin records.where('books.banned = ?', value[0] == 'true') + else + records end end