AlphaNico AlphaNico - 3 years ago 170
Ruby Question

Rails where query chain with or for array input

I want to create a query chain in order to find my bot users given specific filters. Some filters can have multiple values. For example, my "locale" filter can have multiple values (fr_FR, en_US for example).

In this example, I check two locales checkboxes (fr_FR and en_US).

I created a query chain but the output is not what I want:


SELECT "bot_users".* FROM "bot_users" WHERE ("bot_users"."core_bot_id"
= ? AND (locale = 'fr_FR') OR "bot_users"."core_bot_id" = ? AND (locale = 'fr_FR') AND (locale = 'en_EN')) [["core_bot_id", 1],
["core_bot_id", 1]]


I would like something like this:


SELECT "bot_users".* FROM "bot_users" WHERE ("bot_users"."core_bot_id"
= ? AND (locale = 'fr_FR' OR 'en_EN')) [["core_bot_id", 1]]


Here is the code:

@filter = Filter.find_by_letter_id(@letter.id)
$i = 1
$a = 1
$s = 1
query = [{first_name: @filter.first_name}, {last_name: @filter.last_name}, {source: @filter.segment}, {gender: @filter.gender}, {timezone: @filter.timezone}, {locale: @filter.locale}, {created_at: [@filter.creation_date_start, @filter.creation_date_finish]}]
query_chain = BotUser.where(core_bot_id: @bot.id)

query.each do |hash|
hash.each_pair do |key, value|
if value.present? == true
if key.to_s == "timezone"
while $i < value.size do
query_chain = query_chain.where("timezone = ?", value[$i].to_f)
$i += 1
end
elsif key.to_s == "locale"
while $a < value.size do
puts $a.to_s
if $a == 1
query_chain = query_chain.where("locale = ?", value[$a])
else
query_chain = query_chain.or(query_chain.where("locale = ?", value[$a]))
end
$a += 1
end
elsif key.to_s == "gender"
query_chain = query_chain.where("gender = ?", value)
elsif key.to_s == "core_bot_id"
query_chain = query_chain.where("core_bot_id = ?", value)
elsif key.to_s == "created_at"
if value[0].present? == true and value[1].present? == true
query_chain = query_chain.where('created_at BETWEEN ? AND ?', value[0], value[1].end_of_day)
elsif value[0].present? == true
query_chain = query_chain.where('created_at > ?', value[0])
elsif value[1].present? == true
query_chain = query_chain.where('created_at < ?', value[1].end_of_day)
end
else
query_chain = query_chain.where("#{key} = ?", value)
end
end
end
end


UPDATE, trying Jaril method:



Controller:



private

def filter_params
params.fetch(:query, {}).permit(:first_name, :last_name, :timezone, :gender)
end

def set_nb_recipients
@filter = Filter.find_by_letter_id(@letter.id)


filter_params = ActionController::Parameters.new({
query: {
core_bot_id: @bot.id,
first_name: @filter.first_name,
last_name: @filter.last_name,
source: @filter.segment,
gender: @filter.gender,
timezone: @filter.timezone,
locale: @filter.locale,
creation_date_start: @filter.creation_date_start,
creation_date_finish: @filter.creation_date_finish
}
})
query = FilterQuery.new(filter_params)

query = FilterQuery.new(filter_params)
@bot_users = query.execute || BotUser.none

@nb_users = @bot_users.length
end


app/models/filter_query.rb



class FilterQuery

include ActiveModel::Model

attr_accessor :first_name, :last_name, :timezone, :gender, :locale, :core_bot_id, :source, :creation_date_start, :creation_date_finish

validates :gender, inclusion: { in: %w(male female) }

def initialize(params)
super(params)
end

def execute
return false unless valid?
@bot_users = BotUser.where(core_bot_id: core_bot_id)
@bot_users = @bot_users.where('first_name LIKE ?', "#{first_name}%") if first_name.present?
@bot_users = @bot_users.where('last_name LIKE ?', "#{last_name}%") if last_name.present?
@bot_users = @bot_users.where(timezone: timezone) if timezone.present?
@bot_users = @bot_users.where(timezone: locale) if locale.present?
@bot_users = @bot_users.where(gender: gender) if gender.present?
@bot_users = @bot_users.where(source: source) if source.present?
@bot_users = @bot_users.where('created_at BETWEEN ? AND ?', creation_date_start, creation_date_finish) if creation_date_start.present? and creation_date_finish.present?
@bot_users = @bot_users.where('created_at > ?', creation_date_start) if creation_date_start.present? and creation_date_finish.present? == false
@bot_users = @bot_users.where('created_at < ?', creation_date_finish) if creation_date_start.present? == false and creation_date_finish.present?
@bot_users
end

end


Unfortunately, this does not return anything. I'm not sure about the params part, could you help me with that? I store the data in a database and get the params from the object.

Answer Source

I'm with Jaryl.

Less if/elsif. What you're looking for is case. And since a bunch of your queries have similar structure, you can stuff those into the else clause of the case statement.

Less if x? == true. If x has a question mark, then it's already returning a true or false. You don't have to say, if true == true. Just say, if x?. Like, if value[0].present?. Depending on your specific requirements, you may be able to skip the present? part, as well. If you're just trying to guard against nil values, then you could just do if value[0]. However, as engineersmnky points out in the comments, if you want to guard against empty strings, hashes, and arrays - then you'll need to stick with if value[0].present?. And remember, you can always stick your if statement at the end of a single line if you're not going to do an else. Like:

query_chain = query_chain.where('created_at > ?', value[0]) if value[0].present?

Less type conversion (key.to_s). Just compare the key variable to another key. Why convert it to a string?

Less looping. Especially with those iteration variables and value comparisons (while $i < value.size) - yucky! This:

while  $i < value.size do
  query_chain = query_chain.where("timezone = ?", value[$i].to_f)
  $i += 1
end

Is not idiomatic. Better would be:

value.each do |timezone|
  query_chain = query_chain.where("timezone = ?", timezone.to_f)
end

Of course, you could make that query a little less verbose:

value.each do |timezone|
  query_chain = query_chain.where(timezone: timezone.to_f)
end

But, all you're doing in that each loop is converting timezone to_f. So, why not do that in one shot and chain a single query, like:

timezones = value.map{|timezone| timezone.to_f}
query_chain = query_chain.where(timezone: timezones)    

Of course, you could save yourself the temporary variable assignment and just do:

query_chain = query_chain.where(timezone: value.map{|timezone| timezone.to_f})

If you don't mind the long(ish) line.

I like Jaryl's approach. If you want to stick with your current approach, though, it could look something like:

query.each do |hash|
  hash.each_pair do |key, value|
    if value
      case key
      when :timezone
        query_chain = query_chain.where(timezone: value.map{|timezone| timezone.to_f})
      when :created_at
        query_chain = query_chain.where('created_at > ?', value[0]) if value[0]
        query_chain = query_chain.where('created_at < ?', value[1].end_of_day) if value[1]
      else
        query_chain = query_chain.where(key => value)
      end
    end
  end
end
Recommended from our users: Dynamic Network Monitoring from WhatsUp Gold from IPSwitch. Free Download