From fef72a495bafbdf1e28f1d5c6cfc62c40368ee59 Mon Sep 17 00:00:00 2001 From: kalle saas Date: Wed, 10 Mar 2021 07:25:50 +0100 Subject: [PATCH 01/12] add failing specs to test for ransack scopes --- spec/dummy.rb | 8 +++++++- spec/filtering_spec.rb | 24 ++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/spec/dummy.rb b/spec/dummy.rb index 6f0c434..1deccee 100644 --- a/spec/dummy.rb +++ b/spec/dummy.rb @@ -31,6 +31,11 @@ class User < ActiveRecord::Base has_many :notes + scope :created_before, ->(date) { where('created_at < ?', date) } + + def self.ransackable_scopes(auth_object = nil) + %i(created_before) + end end class Note < ActiveRecord::Base @@ -83,7 +88,8 @@ class UsersController < ActionController::Base def index allowed_fields = [ :first_name, :last_name, :created_at, - :notes_created_at, :notes_quantity + :notes_created_at, :notes_quantity, + :created_before ] options = { sort_with_expressions: true } diff --git a/spec/filtering_spec.rb b/spec/filtering_spec.rb index f3bd80a..8d25d0b 100644 --- a/spec/filtering_spec.rb +++ b/spec/filtering_spec.rb @@ -96,6 +96,30 @@ expect(response_json['data'][0]).to have_id(second_user.id.to_s) end end + + context 'returns users filtered by scope' do + let(:params) do + third_user.update(created_at: '2013-01-01') + + { + filter: { created_before: '2013-02-01' } + } + end + + fit 'ensures ransack scopes are working properly' do + ransack = User.ransack({ created_before: '2013-02-01' }) + expected_sql = 'SELECT "users".* FROM "users" WHERE '\ + '(created_at < \'2013-02-01\')' + expect(ransack.result.to_sql).to eq(expected_sql) + end + + fit 'should return only' do + binding.pry + expect(response).to have_http_status(:ok) + expect(response_json['data'].size).to eq(1) + expect(response_json['data'][0]).to have_id(third_user.id.to_s) + end + end end end end From cccf5ed7f96d37a8a0757acdb436f0f17b942105 Mon Sep 17 00:00:00 2001 From: kalle saas Date: Wed, 10 Mar 2021 07:37:13 +0100 Subject: [PATCH 02/12] remove pry reference when commiting --- spec/filtering_spec.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/filtering_spec.rb b/spec/filtering_spec.rb index 8d25d0b..a078de0 100644 --- a/spec/filtering_spec.rb +++ b/spec/filtering_spec.rb @@ -114,7 +114,6 @@ end fit 'should return only' do - binding.pry expect(response).to have_http_status(:ok) expect(response_json['data'].size).to eq(1) expect(response_json['data'][0]).to have_id(third_user.id.to_s) From 62c092d95dacf05aa60a84df6c8b742b34010b33 Mon Sep 17 00:00:00 2001 From: kalle saas Date: Wed, 10 Mar 2021 08:38:54 +0100 Subject: [PATCH 03/12] add allowed_scopes to jsonapi_filter and keep it backwards compatible --- lib/jsonapi/filtering.rb | 16 +++++++++++++--- spec/dummy.rb | 9 +++++++-- spec/filtering_spec.rb | 4 ++-- 3 files changed, 22 insertions(+), 7 deletions(-) diff --git a/lib/jsonapi/filtering.rb b/lib/jsonapi/filtering.rb index 70d47db..7f9e2c0 100644 --- a/lib/jsonapi/filtering.rb +++ b/lib/jsonapi/filtering.rb @@ -32,9 +32,11 @@ def self.extract_attributes_and_predicates(requested_field) # @param allowed_fields [Array] a list of allowed fields to be filtered # @param options [Hash] extra flags to enable/disable features # @return [ActiveRecord::Base] a collection of resources - def jsonapi_filter(resources, allowed_fields, options = {}) + def jsonapi_filter(resources, allowed_fields, *allowed_scopes, options) + options = options || {} + allowed_scopes = (allowed_scopes || []).flatten.map(&:to_s) allowed_fields = allowed_fields.map(&:to_s) - extracted_params = jsonapi_filter_params(allowed_fields) + extracted_params = jsonapi_filter_params(allowed_fields, allowed_scopes) extracted_params[:sorts] = jsonapi_sort_params(allowed_fields, options) resources = resources.ransack(extracted_params) block_given? ? yield(resources) : resources @@ -47,7 +49,7 @@ def jsonapi_filter(resources, allowed_fields, options = {}) # # @param allowed_fields [Array] a list of allowed fields to be filtered # @return [Hash] to be passed to [ActiveRecord::Base#order] - def jsonapi_filter_params(allowed_fields) + def jsonapi_filter_params(allowed_fields, allowed_scopes) filtered = {} requested = params[:filter] || {} allowed_fields = allowed_fields.map(&:to_s) @@ -62,9 +64,17 @@ def jsonapi_filter_params(allowed_fields) to_filter = to_filter.split(',') end + # filter by attributes + # {"first_name_eq"=>"Beau"} if predicates.any? && (field_names - allowed_fields).empty? filtered[requested_field] = to_filter end + + # filter by scopes + # {"created_before"=>"2013-02-01"} + if (field_names - allowed_scopes).empty? + filtered[requested_field] = to_filter + end end filtered diff --git a/spec/dummy.rb b/spec/dummy.rb index 1deccee..5161a36 100644 --- a/spec/dummy.rb +++ b/spec/dummy.rb @@ -88,12 +88,17 @@ class UsersController < ActionController::Base def index allowed_fields = [ :first_name, :last_name, :created_at, - :notes_created_at, :notes_quantity, + :notes_created_at, :notes_quantity + ] + allowed_scopes = [ :created_before ] options = { sort_with_expressions: true } - jsonapi_filter(User.all, allowed_fields, options) do |filtered| + jsonapi_filter(User.all, + allowed_fields, + allowed_scopes, + options) do |filtered| result = filtered.result if params[:sort].to_s.include?('notes_quantity') diff --git a/spec/filtering_spec.rb b/spec/filtering_spec.rb index a078de0..7dddc7c 100644 --- a/spec/filtering_spec.rb +++ b/spec/filtering_spec.rb @@ -106,14 +106,14 @@ } end - fit 'ensures ransack scopes are working properly' do + it 'ensures ransack scopes are working properly' do ransack = User.ransack({ created_before: '2013-02-01' }) expected_sql = 'SELECT "users".* FROM "users" WHERE '\ '(created_at < \'2013-02-01\')' expect(ransack.result.to_sql).to eq(expected_sql) end - fit 'should return only' do + it 'should return only' do expect(response).to have_http_status(:ok) expect(response_json['data'].size).to eq(1) expect(response_json['data'][0]).to have_id(third_user.id.to_s) From e81e3bf9efa9790a60105f1fba1a841140a6ef5b Mon Sep 17 00:00:00 2001 From: kalle saas Date: Wed, 10 Mar 2021 12:54:41 +0100 Subject: [PATCH 04/12] enforce filter by scopes to have exact matches with the allowed_scopes if we encounter an exact match on requested_field with allowed_scopes do not continue to check for additional filter with attribute and predicate. --- lib/jsonapi/filtering.rb | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/lib/jsonapi/filtering.rb b/lib/jsonapi/filtering.rb index 7f9e2c0..97f7347 100644 --- a/lib/jsonapi/filtering.rb +++ b/lib/jsonapi/filtering.rb @@ -64,15 +64,21 @@ def jsonapi_filter_params(allowed_fields, allowed_scopes) to_filter = to_filter.split(',') end - # filter by attributes - # {"first_name_eq"=>"Beau"} - if predicates.any? && (field_names - allowed_fields).empty? + # filter by scopes expects an exact match + # with the `allowed_scopes`. Predicates can be a part of named scopes + # and should be handled first + # Make sure to move to the next after a match + # {"created_before"=>"2013-02-01"} + # {"created_before_gt"=>"2013-02-01"} + if allowed_scopes.include?(requested_field) filtered[requested_field] = to_filter + next end - # filter by scopes - # {"created_before"=>"2013-02-01"} - if (field_names - allowed_scopes).empty? + + # filter by attributes + # {"first_name_eq"=>"Beau"} + if predicates.any? && (field_names - allowed_fields).empty? filtered[requested_field] = to_filter end end From c55bd54afbaecb6bea822d7952846a60ee637724 Mon Sep 17 00:00:00 2001 From: kalle saas Date: Wed, 10 Mar 2021 13:32:03 +0100 Subject: [PATCH 05/12] add failing tests to show that we cannot filter by e.g. `notes_count_eq` --- spec/dummy.rb | 1 + spec/filtering_spec.rb | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/spec/dummy.rb b/spec/dummy.rb index 6f0c434..4babd87 100644 --- a/spec/dummy.rb +++ b/spec/dummy.rb @@ -18,6 +18,7 @@ create_table :users, force: true do |t| t.string :first_name t.string :last_name + t.integer :notes_count, default: 0 t.timestamps end diff --git a/spec/filtering_spec.rb b/spec/filtering_spec.rb index f3bd80a..13e7daf 100644 --- a/spec/filtering_spec.rb +++ b/spec/filtering_spec.rb @@ -85,6 +85,25 @@ end end + context 'returns users by counter_cache' do + let(:params) do + second_user.update(notes_count: 1) + { + filter: { notes_count_eq: 1 } + } + end + + fit do + expect(first_user.notes_count).to eq(0) + expect(second_user.notes_count).to eq(1) + expect(third_user.notes_count).to eq(0) + + expect(response).to have_http_status(:ok) + expect(response_json['data'].size).to eq(1) + expect(response_json['data'][0]).to have_id(second_user.id.to_s) + end + end + context 'returns sorted users by notes' do let(:params) do { sort: '-notes_created_at' } From 89e5da38aefbbd86b10286f7b7f132406fa9c0ba Mon Sep 17 00:00:00 2001 From: kalle saas Date: Wed, 10 Mar 2021 13:44:34 +0100 Subject: [PATCH 06/12] add failing spec for `extract_attributes_and_predicates` implementation --- spec/filtering_spec.rb | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/spec/filtering_spec.rb b/spec/filtering_spec.rb index 13e7daf..7a4ef2b 100644 --- a/spec/filtering_spec.rb +++ b/spec/filtering_spec.rb @@ -12,6 +12,16 @@ end end + context 'attributes with underscore in name' do + it 'detect _' do + attributes, predicates = JSONAPI::Filtering + .extract_attributes_and_predicates('notes_count_eq') + expect(attributes).to eq(['notes_count']) + expect(predicates.size).to eq(1) + expect(predicates[0].name).to eq('eq') + end + end + context 'mixed predicates' do it 'extracts in order' do attributes, predicates = JSONAPI::Filtering @@ -93,7 +103,7 @@ } end - fit do + it do expect(first_user.notes_count).to eq(0) expect(second_user.notes_count).to eq(1) expect(third_user.notes_count).to eq(0) From 92dbda51801dee777ec361293c87da46e0bc834e Mon Sep 17 00:00:00 2001 From: kalle saas Date: Wed, 10 Mar 2021 14:12:22 +0100 Subject: [PATCH 07/12] add notes_count to allowed_fields --- spec/dummy.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spec/dummy.rb b/spec/dummy.rb index 4babd87..c5a8ffe 100644 --- a/spec/dummy.rb +++ b/spec/dummy.rb @@ -84,7 +84,8 @@ class UsersController < ActionController::Base def index allowed_fields = [ :first_name, :last_name, :created_at, - :notes_created_at, :notes_quantity + :notes_created_at, :notes_quantity, + :notes_count ] options = { sort_with_expressions: true } From debfec8b9078403e8164b06faf9c41092d82d05c Mon Sep 17 00:00:00 2001 From: kalle saas Date: Wed, 10 Mar 2021 14:13:03 +0100 Subject: [PATCH 08/12] add test to ensure ransack works as expected --- spec/filtering_spec.rb | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/spec/filtering_spec.rb b/spec/filtering_spec.rb index 7a4ef2b..34eb6d0 100644 --- a/spec/filtering_spec.rb +++ b/spec/filtering_spec.rb @@ -12,7 +12,7 @@ end end - context 'attributes with underscore in name' do + context 'handle attributes with underscore in name' do it 'detect _' do attributes, predicates = JSONAPI::Filtering .extract_attributes_and_predicates('notes_count_eq') @@ -103,6 +103,13 @@ } end + it 'ensures ransack scopes work properly' do + ransack = User.ransack(notes_count_eq: 1) + expected_sql = 'SELECT "users".* FROM "users" WHERE '\ + '"users"."notes_count" = 1' + expect(ransack.result.to_sql).to eq(expected_sql) + end + it do expect(first_user.notes_count).to eq(0) expect(second_user.notes_count).to eq(1) From d6906ec934b55ccf5608a001eed9efaee765cc87 Mon Sep 17 00:00:00 2001 From: kalle saas Date: Wed, 10 Mar 2021 14:31:07 +0100 Subject: [PATCH 09/12] fix failing tests to filter by _count attributes and other --- lib/jsonapi/filtering.rb | 11 +++++++---- spec/filtering_spec.rb | 11 +++++++---- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/lib/jsonapi/filtering.rb b/lib/jsonapi/filtering.rb index 70d47db..c943e90 100644 --- a/lib/jsonapi/filtering.rb +++ b/lib/jsonapi/filtering.rb @@ -8,16 +8,19 @@ module Filtering # # @param requested_field [String] the field to parse # @return [Array] with the fields and the predicate - def self.extract_attributes_and_predicates(requested_field) + def self.extract_attributes_and_predicates(requested_field, allowed_fields) predicates = [] field_name = requested_field.to_s.dup while Ransack::Predicate.detect_from_string(field_name).present? do + # break if we have an exect match with an allowed_fields + # we do not want to pick apart the string further + break if allowed_fields.include?(field_name) + predicate = Ransack::Predicate .detect_and_strip_from_string!(field_name) predicates << Ransack::Predicate.named(predicate) end - [field_name.split(/_and_|_or_/), predicates.reverse] end @@ -54,7 +57,7 @@ def jsonapi_filter_params(allowed_fields) requested.each_pair do |requested_field, to_filter| field_names, predicates = JSONAPI::Filtering - .extract_attributes_and_predicates(requested_field) + .extract_attributes_and_predicates(requested_field, allowed_fields) wants_array = predicates.any? && predicates.map(&:wants_array).any? @@ -88,7 +91,7 @@ def jsonapi_sort_params(allowed_fields, options = {}) end field_names, predicates = JSONAPI::Filtering - .extract_attributes_and_predicates(requested_field) + .extract_attributes_and_predicates(requested_field, allowed_fields) next unless (field_names - allowed_fields).empty? next if !options[:sort_with_expressions] && predicates.any? diff --git a/spec/filtering_spec.rb b/spec/filtering_spec.rb index 34eb6d0..2ea3bf1 100644 --- a/spec/filtering_spec.rb +++ b/spec/filtering_spec.rb @@ -4,8 +4,11 @@ describe '#extract_attributes_and_predicate' do context 'mixed attributes (and/or)' do it 'extracts ANDs' do - attributes, predicates = JSONAPI::Filtering - .extract_attributes_and_predicates('attr1_and_attr2_eq') + attributes, predicates = + JSONAPI::Filtering.extract_attributes_and_predicates( + 'attr1_and_attr2_eq', + ['attr1', 'attr2'] + ) expect(attributes).to eq(['attr1', 'attr2']) expect(predicates.size).to eq(1) expect(predicates[0].name).to eq('eq') @@ -15,7 +18,7 @@ context 'handle attributes with underscore in name' do it 'detect _' do attributes, predicates = JSONAPI::Filtering - .extract_attributes_and_predicates('notes_count_eq') + .extract_attributes_and_predicates('notes_count_eq', ['notes_count']) expect(attributes).to eq(['notes_count']) expect(predicates.size).to eq(1) expect(predicates[0].name).to eq('eq') @@ -25,7 +28,7 @@ context 'mixed predicates' do it 'extracts in order' do attributes, predicates = JSONAPI::Filtering - .extract_attributes_and_predicates('attr1_sum_eq') + .extract_attributes_and_predicates('attr1_sum_eq', ['attr1']) expect(attributes).to eq(['attr1']) expect(predicates.size).to eq(2) expect(predicates[0].name).to eq('sum') From 2b4fb3c556d04128ab8b44d4db8b02c6e5fb5a6c Mon Sep 17 00:00:00 2001 From: Justin Rumpler Date: Thu, 31 Mar 2022 13:11:11 +0200 Subject: [PATCH 10/12] fix deprecation warnings --- lib/jsonapi/rails.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/jsonapi/rails.rb b/lib/jsonapi/rails.rb index 8d29d17..a27b515 100644 --- a/lib/jsonapi/rails.rb +++ b/lib/jsonapi/rails.rb @@ -37,7 +37,7 @@ def self.install! # @return [NilClass] def self.add_errors_renderer! ActionController::Renderers.add(:jsonapi_errors) do |resource, options| - self.content_type ||= Mime[:jsonapi] + self.content_type = Mime[:jsonapi] if self.media_type.nil? many = JSONAPI::Rails.is_collection?(resource, options[:is_collection]) resource = [resource] unless many @@ -90,7 +90,7 @@ def self.add_errors_renderer! # @return [NilClass] def self.add_renderer! ActionController::Renderers.add(:jsonapi) do |resource, options| - self.content_type ||= Mime[:jsonapi] + self.content_type = Mime[:jsonapi] if self.media_type.nil? JSONAPI_METHODS_MAPPING.to_a[0..1].each do |opt, method_name| next unless respond_to?(method_name, true) From 43ed04a8a8f1e73234479ec4bd049b450399c1b0 Mon Sep 17 00:00:00 2001 From: Justin Rumpler Date: Tue, 5 Apr 2022 14:54:07 +0200 Subject: [PATCH 11/12] add specs to ensure mime type is still set correctly --- spec/errors_spec.rb | 1 + spec/fetching_spec.rb | 1 + 2 files changed, 2 insertions(+) diff --git a/spec/errors_spec.rb b/spec/errors_spec.rb index da3d0f5..1dd6d11 100644 --- a/spec/errors_spec.rb +++ b/spec/errors_spec.rb @@ -31,6 +31,7 @@ it do expect(response).to have_http_status(:unprocessable_entity) + expect(response.media_type).to eq("application/vnd.api+json") expect(response_json['errors'].size).to eq(1) expect(response_json['errors'][0]['status']).to eq('422') expect(response_json['errors'][0]['title']) diff --git a/spec/fetching_spec.rb b/spec/fetching_spec.rb index a5382e7..249cf71 100644 --- a/spec/fetching_spec.rb +++ b/spec/fetching_spec.rb @@ -25,6 +25,7 @@ it do expect(response).to have_http_status(:ok) expect(response_json['data'].size).to eq(users.size) + expect(response.media_type).to eq("application/vnd.api+json") response_json['data'].each do |item| user = users.detect { |u| u.id == item['id'].to_i } From 7f2aa5ded6f7b97c095592fe78f002f19208d538 Mon Sep 17 00:00:00 2001 From: Justin Rumpler Date: Tue, 5 Apr 2022 16:23:11 +0200 Subject: [PATCH 12/12] corrected StringLiterals --- .ruby-version | 1 + lib/jsonapi/rails.rb | 2 +- spec/errors_spec.rb | 2 +- spec/fetching_spec.rb | 2 +- 4 files changed, 4 insertions(+), 3 deletions(-) create mode 100644 .ruby-version diff --git a/.ruby-version b/.ruby-version new file mode 100644 index 0000000..d48d370 --- /dev/null +++ b/.ruby-version @@ -0,0 +1 @@ +2.6.9 diff --git a/lib/jsonapi/rails.rb b/lib/jsonapi/rails.rb index a27b515..c221268 100644 --- a/lib/jsonapi/rails.rb +++ b/lib/jsonapi/rails.rb @@ -47,7 +47,7 @@ def self.add_errors_renderer! ) unless resource.is_a?(ActiveModel::Errors) errors = [] - model = resource.instance_variable_get('@base') + model = resource.instance_variable_get(:@base) if respond_to?(:jsonapi_serializer_class, true) model_serializer = jsonapi_serializer_class(model, false) diff --git a/spec/errors_spec.rb b/spec/errors_spec.rb index 1dd6d11..38cf978 100644 --- a/spec/errors_spec.rb +++ b/spec/errors_spec.rb @@ -31,7 +31,7 @@ it do expect(response).to have_http_status(:unprocessable_entity) - expect(response.media_type).to eq("application/vnd.api+json") + expect(response.media_type).to eq('application/vnd.api+json') expect(response_json['errors'].size).to eq(1) expect(response_json['errors'][0]['status']).to eq('422') expect(response_json['errors'][0]['title']) diff --git a/spec/fetching_spec.rb b/spec/fetching_spec.rb index 249cf71..513f185 100644 --- a/spec/fetching_spec.rb +++ b/spec/fetching_spec.rb @@ -25,7 +25,7 @@ it do expect(response).to have_http_status(:ok) expect(response_json['data'].size).to eq(users.size) - expect(response.media_type).to eq("application/vnd.api+json") + expect(response.media_type).to eq('application/vnd.api+json') response_json['data'].each do |item| user = users.detect { |u| u.id == item['id'].to_i }