searchlight
searchlight copied to clipboard
Do not remove empty values in a non-empty Array.
Hey @nathanl,
Wanted to get your feedback on this.
After upgrading from v3 to v4, we noticed some of our searches were not working.
We use an Array for date ranges, first element is Start Date and second element is End Date. We also allow for open-ended date ranges by leaving one of those nil.
So it would look like this for >= Jan 1, 2024:
options[ :created_between ] = [ "Jan 1, 2024", nil ]
In v3 and before, that nil value would be left in there.
In v4, that nil value is being removed and the Array becomes [ "Jan 1, 2024" ].
This occurs in the excluding_empties method:
if value.instance_of?(Array)
output[key] = value.reject { |v| empty?(v) }
end
Our thinking is to only reject an Array that is completely empty or filled with nil or empty Strings "".
We forked it to look something like this:
if value.instance_of?(Array)
output[key] = nil if output[key].all?(&:blank?) # If Array is entirely empty (i.e. `nil` or empty Strings only) then remove it.
end
Two things:
- Was this an intentional change from v3 to v4? If not, we can restore how it worked.
- If this was intentional then would you be open to a config to change the behaviour here and to avoid removing
blank?values from a non-blank Array?
We're using our fork for now so no rush but let me know your thoughts.
Thanks.
Hey Joshua. Glad you figured out the problem you're having. Given that the last release was several years ago and I haven't used Ruby in about as long, I'm very hesitant to change anything at this point.
I suggest you either keep using your fork (I doubt this repo will ever change) or else use a value like :empty instead of nil. What do you think?
I honestly don't remember the reason for this change but I assume others are relying on the v4 behavior.
Totally fair and I really appreciate the response, regardless. I know what it's like keeping legacy open source projects around. Ha.
I think we'll continue using our fork for now, especially since this lib won't be changing so it's not like we're missing out on "upstream" changes or version bumps.
If we circle back to this and have more time to put in to keep it updated, etc I'll let you know and maybe we can take it off your hands, maintenance-wise.
Gonna leave this open as a record in case it catches anybody else outs, if you're okay with that.