Reputation: 290
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
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
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.
Upvotes: 3
Views: 1535
Reputation: 20263
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
Here's a slightly different implementation of the Jaryl approach...
class FilterQuery
attr_accessor :first_name,
:last_name,
:timezone,
:gender,
:locale,
:core_bot_id,
:source,
:creation_date_start,
:creation_date_finish
def initialize(params)
params.each{|k,v| send("#{k}=",v)}
end
def execute
return false unless valid?
@bot_users = BotUser.where(core_bot_id: core_bot_id)
[:first_name, :last_name].each do |var_sym|
val = send(var_sym)
@bot_users = @bot_users.where("#{var_sym} LIKE ?", "#{val}%") if val.present?
end
[:timezone, :locale, :gender, :source].each do |var_sym|
val = send(var_sym)
@bot_users = @bot_users.where(var_sym => val) if val.present?
end
@bot_users = @bot_users.where('created_at > ?', creation_date_start) if creation_date_start.present?
@bot_users = @bot_users.where('created_at < ?', creation_date_finish) if creation_date_finish.present?
@bot_users
end
private
def valid?
%w(male female).include? gender
end
end
Upvotes: 3
Reputation: 2601
Ideally, you would want to do all these in a query class. Also, try to tone down on the if/else yeah?
Here's a sample of what that would look like, it's incomplete, but I've thrown in a validation for you for good measure:
class FilterQuery
include ActiveModel::Model
attr_accessor :first_name, :last_name, :timezone, :gender
validates :gender, inclusion: { in: %w(male female) }
def initialize(params)
super(params)
end
def execute
return false unless valid?
@bot_users = BotUser.all
@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(gender: gender) if gender.present?
@bot_users
end
end
To use it, plop this into your controller:
def index
query = FilterQuery.new(filter_params)
@bot_users = query.execute || BotUser.none
end
private
def filter_params
params.fetch(:query, {}).permit(:first_name, :last_name, :timezone, :gender)
end
Upvotes: 3