Rails where query chain with or for array input

Question

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.

the SQL you are looking for is IN() and can easily be where(locale: ['fr_FR','en_EN']) which will produce locale IN('fr_FR','en_EN') which is essentially an OR statement — engineersmnky
– engineersmnky, Commented Sep 29, 2017 at 12:58
Oh! This works perfectly well, thank you very much!! I didn't know about this "IN" feature... — nico_lrx
– nico_lrx, Commented Sep 29, 2017 at 13:09
Did you read my comments on @Jaryl's post execute method should end with @bot_users otherwise you will get a nil response unless (in your case) this condition is true if creation_date_start.present? == false and creation_date_finish.present? — engineersmnky
– engineersmnky, Commented Sep 29, 2017 at 14:03
I'm not sure what you mean by adding @bot_users at the end of the method? Just like I put it now (see updated question) — nico_lrx
– nico_lrx, Commented Sep 29, 2017 at 14:06

jvillian · Accepted Answer · 2017-09-30 18:04:06Z

3

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

edited Sep 30, 2017 at 18:04

answered Sep 29, 2017 at 13:38

jvillian

20.3k5 gold badges33 silver badges45 bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

jvillian Over a year ago

Hope it helps. Glad to see you trying out Jaryl's approach! Remember, if gender. Not, if gender.present?.

engineersmnky Over a year ago

@jvillian present? and truthy/falsey are not the same. present? is equal to !blank? where blank? is Object specific and can be delegated to empty? as well. e.g. "".present? #=> false, [].present? #=> false, {}.present? #=> false where as standard ruby would state that all of those Objects are truthy

jvillian Over a year ago

@engineersmnky - Thank you for the correction! I updated the answer with attribution. Hopefully, I got it right this time.

nico_lrx Over a year ago

I couldn't make it work with Jaryl's solution (my understanding of strong params is probably not sufficient enough even if his solution seems really good). Your solution works perfectly well so I mark it as the answer. Again thanks for the explanations!

Jaryl · Accepted Answer · 2017-09-29 15:21:37Z

3

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

edited Sep 29, 2017 at 15:21

answered Sep 29, 2017 at 13:10

Jaryl

2,6012 gold badges26 silver badges33 bronze badges

6 Comments

engineersmnky Over a year ago

Only problem is if gender is not present execute will return nil (regardless of the existence of other parameters). Place @bot_users as the last line of execute to resolve this

engineersmnky Over a year ago

Also params.fetch(:query, {}).permit(:first_name, :last_name, :timezone, :gender) will have an issue if :query does not exist because permit is not a Hash method.you could change this to ActiveSupport::HashWithIndifferentAccess.new

nico_lrx Over a year ago

Could you explain this point please? When I try it, there is no response (no BotUser query performed)

Jaryl Over a year ago

@engineersmnky that's true, missed that out and didn't test it, but glad he's got it fixed.

nico_lrx Over a year ago

I'm not sure my code fetches the params. How can I wrap the params into :query?

|

Collectives™ on Stack Overflow

Rails where query chain with or for array input

UPDATE, trying Jaril method:

Controller:

app/models/filter_query.rb

2 Answers 2

4 Comments

6 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

UPDATE, trying Jaril method:

Controller:

app/models/filter_query.rb

2 Answers 2

4 Comments

6 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related