nico_lrx
nico_lrx

Reputation: 290

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.

Upvotes: 3

Views: 1535

Answers (2)

jvillian
jvillian

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

Jaryl
Jaryl

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

Related Questions