activerecord-hackery/ransack

Filtering by scopes with an array as an argument raises ArgumentError

ohaibbq opened this issue ยท 15 comments

Assuming:

class Person < ActiveRecord::Base
    scope :domestic, ->(countries) { where(country: countries) }
    def self.ransackable_scopes(auth_object=nil); [:domestic]; end
end

Trying to filter by a list of countries:

Person.search(country: ['US', 'JP'])

Will raise ArgumentError: wrong number of arguments (2 for 1)

Again, we are able to format the input in order to avoid this.

Person.search(country: [['US', 'JP']])

This avoids the issue as it seems to be a problem with splatting the args.

I'm pretty sure the fix for this is not to splat the args if the arity is 1.

I was looking in to this for a bit, here's what I found..

ActiveRecord::Base#scope defines a method that takes a variable number of arguments, it doesn't seem possible that we can check the arity of the Proc that's passed in to scope.

Context#chain_scope inspects the scope arity but it seems to always be -1. I'm not sure why it does this and there aren't any tests around it.

I'm not sure how to continue as I can't think of a way to design the API to accept this input. It seems like we might just have to wrap the array.

/cc @avit and @glebm

For now, a workaround is to define a class method:

def self.domestic(countries) 
  where(country: countries)
end

Wrapping the array might mean losing compatibility with forms as is. Instead, perhaps scope can be patched to check for Proc#arity, similar to this gist. Then PR to Rails to define class methods with the correct arity in the first place (instead of *args).

Class method seems to have the same issues. Will investigate more later.

I've just encountered this (retrofitting an old metasearch project) - are there any obvious workarounds or alternatives that I may be missing?

Double wrapping the array does allow executing the search, but as @glebm points out above, this results in a search object that's not compatible with the form helpers. I considered wrapping, performing the search, and then unwrapping what's passed to the view, but I'm not sure this is feasible given what the search object exposes.

@rahim Late reply, but we were modifying a duped version of the params so that the input would remain compatible for the form helpers

@jonatack Thanks, I'll take a look at this a bit more tonight.

fluke commented

How would one get around this? Is there a viable alternative method.

lyc4n commented

I tried used splat and then flatten the argument as a workaround

class Person < ActiveRecord::Base
    scope :domestic, ->(*countries) do 
       flatten_countries = countries.flatten
       where(country: flatten_countries)
     end
    def self.ransackable_scopes(auth_object=nil); [:domestic]; end
end
Liooo commented

We could have a logic to pass the argument without splat-ing depending on the arity here.

https://github.com/activerecord-hackery/ransack/blob/master/lib/ransack/context.rb#L49-L53

But when the method is given as an ActiveRecord scope (instead of class method), there's no way to access the arity (rails/rails#28198).

Though rails doc recommends using class method when having arguments, so far @lyc4n 's solution suffices I guess.

Using a class method is the preferred way to accept arguments for scopes. These methods will still be accessible on the association objects

https://guides.rubyonrails.org/active_record_querying.html#passing-in-arguments

Liooo commented

or there could be an API like ransackable_scopes_skip_splat_args to let users specify scopes called without argument splat. Would you guys accept PR if I sent?

@gregmolnar @seanfcarroll

@Liooo sure that would be fine, please add additional tests to cover it.

By the way - see this https://github.com/danmcclain/postgres_ext

Closed due to inactivity

Ecco commented

Hit the same problem. Does it count as activity? ๐Ÿ˜„

cpgo commented

Any updates on this?
Hit the same issue today, worked arround it with the splat trick mentioned above.