heartcombo/has_scope

Hash sends "nil" values to scope even if allow_blank is not set to true

murtazabasrai opened this issue · 6 comments

My setup:
User has 3 select inputs to choose different filter values (filter1, filter2, filter3).
I have nested params which look like:

{"utf8"=>"✓", "filter"=>{"show_discontinued"=>"true", "filter1"=>"value1", "filter2"=>"value2", "filter3"=>"value3"}, "commit"=>"", "page"=>"", "size"=>"50", "sort"=>"", "direction"=>""}

The scopes are set on filter values filter1, filter2, filter3

To pass the nested hash value to scope, I use in: filter which works perfectly fine:
controller:

has_scope :filter1, in: :filter

model:

scope :filter1, -> value { where('table.association_id = ?', value) }

printing current_scopes:

{:filter=>{"show_discontinued"=>"true", "filter1"=>"18"}}

Till here things work fine.

This issue starts now:
When user selects the "All" option in the select input (which has a blank value), the nil value is passed to scope which results in following query

WHERE (table.association_id = NULL)

So the query looks for null matches against the column defined in the scope. I don't have any null values for association_id so the it returns 0 records.

params look like:

{"utf8"=>"✓", "filter"=>{"filter1"=>"", "filter2"=>"", "filter3"=>""}, "commit"=>"", "page"=>"", "size"=>"50", "sort"=>"", "direction"=>""}

printing current_scopes:

{:filter=>{"show_discontinued"=>"true"}}

This behaviour is only witnessed when filter contains other values as above. If I remove all filters, then current_scopes becomes {} which works fine.

Based on the Readme, I expect that when the value is nil or not present, it should not call the scope - unless allow_blank is set to true.
This works fine for values which have string or integer type and not hash.

Let me know if I my understanding is not correct and there's some way to make it work.

Note: Currently, I am checking for nil values in the scope itself as a workaround:

scope :filter1, -> value { where('table.association_id = ?', value)  if value.present? }

Hey @murtazabasrai I opened #118 to hopefully fix this issue, and prevent calling the scopes unless you specifically set them up with allow_blank: true.

Let me know if you are still able to test it out in your app, I plan to merge it and release a new version in a few days if all goes well. Thanks.

@carlosantoniodasilva any chance this (^^) will be merged & fixed? Seems that I am experiencing the same issue

@nasaabg can you test the branch and see if it works for you? That'll help confirm it's what we need and we can get it merged at least. Thanks.

@carlosantoniodasilva I have added my outcome under PR

Do we have any update here?

@nasaabg nope :), I don't have an ETA but I have bumped it in my list on the things to look at soon. Thanks.