Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow negation #8

Merged
merged 2 commits into from
Mar 9, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Gemfile.lock
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
PATH
remote: .
specs:
filterable-by (0.5.3)
filterable-by (0.6.0)
activerecord
activesupport

Expand Down
16 changes: 11 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,15 @@ class Comment < ActiveRecord::Base
belongs_to :post

filterable_by :post_id, :user_id
filterable_by :post_author_id do |scope, value|
scope.joins(:posts).where(:'posts.author_id' => value)
filterable_by :post_author_id do |value|
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so the big change is that you don't now need scope as a block argument, but the old method is still supported (although deprecated)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will we still be able to handle when value is nil cases?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

value cannot be nil, these blocks are only run unless value.bank?

joins(:posts).where(:'posts.author_id' => value)
end
filterable_by :only do |scope, value, **opts|
filterable_by :only do |value, **opts|
case value
when 'mine'
scope.where(user_id: opts[:user_id]) if opts[:user_id]
where(user_id: opts[:user_id]) if opts[:user_id]
else
scope
all
end
end
end
Expand All @@ -39,6 +39,12 @@ Simple use cases:
Comment.filter_by({ 'post_id' => '1' })
# => WHERE post_id = 1

Comment.filter_by({ 'post_id' => ['1', '2'] })
# => WHERE post_id IN (1, 2)

Comment.filter_by({ 'post_id_not' => '3' })
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the other change is that you now can append _not and exclude something

# => WHERE post_id != 3

Comment.filter_by({ 'user_id' => '2', 'ignored' => '3' })
# => WHERE user_id = 2

Expand Down
2 changes: 1 addition & 1 deletion filterable-by.gemspec
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Gem::Specification.new do |s|
s.name = 'filterable-by'
s.version = '0.5.3'
s.version = '0.6.0'
s.authors = ['Dimitrij Denissenko']
s.email = ['dimitrij@blacksquaremedia.com']
s.summary = 'Generate white-listed filter scopes from URL parameter values'
Expand Down
37 changes: 30 additions & 7 deletions lib/filterable_by.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,28 @@ def self.normalize(value)
end
end

def self.merge(scope, unscoped, hash, name, **opts, &block)
key = name
positive = normalize(hash[key]) if hash.key?(key)
if positive.present?
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andyborn here we only apply block/limit scope if params['post_author_id'] is present

sub = block.arity == 2 ? yield(unscoped, positive, **opts) : yield(positive, **opts)
return nil unless sub

scope = scope.merge(sub)
end

key = "#{name}_not"
negative = normalize(hash[key]) if hash.key?(key)
if negative.present?
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same with params['post_author_id_not']

sub = block.arity == 2 ? yield(unscoped, negative, **opts) : yield(negative, **opts)
return nil unless sub

scope = scope.merge(sub.invert_where)
end

scope
end

module ClassMethods
def self.extended(base) # :nodoc:
base.class_attribute :_filterable_by_config, instance_accessor: false, instance_predicate: false
Expand All @@ -26,8 +48,12 @@ def inherited(base) # :nodoc:
end

def filterable_by(*names, &block)
if block && block.arity > 1
ActiveSupport::Deprecation.warn('using scope in filterable_by blocks is deprecated. Please use filterable_by(:x) {|val| where(field: val) } instead.')
end

names.each do |name|
_filterable_by_config[name.to_s] = block || ->(scope, value, **) { scope.where(name.to_sym => value) }
_filterable_by_config[name.to_s] = block || ->(value, **) { where(name.to_sym => value) }
end
end

Expand All @@ -38,16 +64,13 @@ def filter_by(hash = nil, **opts)
hash = opts
opts = {}
end

scope = all
return scope unless hash.respond_to?(:key?) && hash.respond_to?(:[])

_filterable_by_config.each do |name, block|
next unless hash.key?(name)

value = FilterableBy.normalize(hash[name])
next if value.blank?

scope = block.call(scope, value, **opts)
scope = FilterableBy.merge(scope, unscoped, hash, name, **opts, &block)
break unless scope
end

scope || none
Expand Down
18 changes: 16 additions & 2 deletions spec/active_record/filterable_by_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@
let(:bpost) { POSTS[:bobs] }

it 'has config' do
expect(Comment.send(:_filterable_by_config).count).to eq(3)
expect(Rating.send(:_filterable_by_config).count).to eq(2)
expect(Comment.send(:_filterable_by_config).count).to eq(5)
expect(Rating.send(:_filterable_by_config).count).to eq(4)
expect(Post.send(:_filterable_by_config).count).to eq(2)
end

Expand Down Expand Up @@ -50,6 +50,14 @@
expect(scope.pluck(:title)).to match_array(%w[AB BB])
end

it 'generates negated scopes' do
expect(Comment.filter_by('author_id_not' => alice.id).pluck(:title)).to match_array(%w[BA BB])
expect(Comment.filter_by('author_id_not' => [alice.id, bob.id]).pluck(:title)).to match_array(%w[])
expect(Comment.filter_by('post_id_not' => apost.id).pluck(:title)).to match_array(%w[AB BB])
expect(Comment.filter_by('post_author_id_not' => alice.id).pluck(:title)).to match_array(%w[AB BB])
expect(Comment.filter_by('author_id' => bob.id, 'post_id_not' => bpost.id).pluck(:title)).to match_array(['BA'])
end

it 'combines with other scopes' do
scope = Comment.where(author_id: alice.id).filter_by('post_id' => apost.id)
expect(scope.pluck(:title)).to match_array(['AA'])
Expand Down Expand Up @@ -77,4 +85,10 @@
expect(Post.filter_by('post_id' => bpost.id).count).to eq(2)
expect(Rating.filter_by('post_author_id' => bob.id).count).to eq(1)
end

it 'supports deprecated scoping' do
expect(Comment.filter_by('deprecated' => alice.id).pluck(:title)).to match_array(%w[AA AB])
expect(Comment.filter_by('deprecated_with_opts' => alice.id).pluck(:title)).to match_array(%w[AA AB])
expect(Comment.filter_by('deprecated_not' => alice.id).pluck(:title)).to match_array(%w[BA BB])
end
end
19 changes: 14 additions & 5 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,12 @@ class Post < ActiveRecord::Base
belongs_to :author

filterable_by :author_id
filterable_by :only do |scope, value, **opts|
filterable_by :only do |value, **opts|
case value
when 'me'
scope.where(author_id: opts[:user_id]) if opts[:user_id]
where(author_id: opts[:user_id]) if opts[:user_id]
else
scope
all
end
end
end
Expand All @@ -43,11 +43,20 @@ class Feedback < ActiveRecord::Base
belongs_to :post

filterable_by :post_id, :author_id

ActiveSupport::Deprecation.silence do
filterable_by :deprecated do |scope, value|
scope.where(author_id: value)
end
filterable_by :deprecated_with_opts do |scope, value, **_opts|
scope.where(author_id: value)
end
end
end

class Comment < Feedback
filterable_by :post_author_id do |scope, value|
scope.joins(:post).where(Post.arel_table[:author_id].eq(value))
filterable_by :post_author_id do |value|
joins(:post).where(Post.arel_table[:author_id].eq(value))
end
end

Expand Down