CODEWITHSUNDEEP
ruby-on-railsrubypostgresqlruby-on-rails-6.1
Shruikan
Shruikan

Reputation: 99

Rails where method with any created_at

I've got a method like this:

def transfers(material, capacity, category, created_at)
  transfers_range = Transfer.first.created_at..Transfer.last.created_at if created_at.nil?
  Transfer.where(
    sender_dispensary_id: id,
    status: %i[dispatched released returned],
    capacity: capacity,
    material: material,
    created_at: created_at.nil? ? transfers_range : Time.given_month_range(created_at)
  ).or(
    Transfer.where(
      receiver_dispensary_id: id,
      status: %i[dispatched released returned],
      capacity: capacity,
      material_id: material,
      created_at: created_at.nil? ? transfers_range : Time.given_month_range(created_at)
    )
  )
end

It's working, but is there a way to avoid query for transfer_range? I mean... If created_at == nil, then this function should skip created_at column, like it was not included in query

Upvotes: 0

Views: 128

Answers (3)

Schwern
Schwern

Reputation: 164679

You can consolidate the created_at query logic so it's in one place and you don't have to repeat it.

  created_at = if created_at.nil?
    Transfer.first.created_at..Transfer.last.created_at
  else
    Time.given_month_range(created_at)
  end

You also don't need to repeat the whole where condition twice. You want the equivalent of...

where status in ('dispatched', 'released', 'returned')
  and capacity = ?
  and material = ?
  and created_at = ?
  and (
    sender_dispensary_id = ?
    or
    receiver_dispensary_id = ?
  )

You do that like so:

  Transfer
    .where(
      status: %i[dispatched released returned],
      capacity: capacity,
      material: material,
      created_at: created_at
    )
    .where(
      Transfer
        .where(receiver_dispensary_id: id)
          .or(Transfer.where(sender_dispensary_id: id))
    )
  )

You can make this even more concise, and hide the details, by putting the logic into a scopes.

class Transfer < Application
  scope :by_dispensary_id ->(id) {
    where(receiver_dispensary_id: id)
      .or(where(sender_dispensary_id: id))
  }

  scope :by_created_month ->(time) {
    if time.nil?
      where(created_at: first.created_at..last.created_at)
    else
      where(created_at: Time.given_month_range(time))
    end
  }
end

And now the query is much simpler.

  Transfer
    .by_dispensary_id(id)
    .by_created_month(created_at)
    .where(
      status: %i[dispatched released returned],
      capacity: capacity,
      material: material
    )
  )

Upvotes: 1

spickermann
spickermann

Reputation: 106802

When created_at is nil then transfers_range basically covers all transfers and therefore the condition is pointless and I simple would query by created_at in that case.

def transfers(material, capacity, category, created_at)
  transfers = Transfer
    .where(status: %i[dispatched released returned], capacity: capacity, material_id: material)
    .where('sender_dispensary_id = :id OR receiver_dispensary_id = :id', id: id)

  transfer = transfer.where(created_at: Time.given_month_range(created_at)) if created_at

  transfer
end

Upvotes: 1

Sachin Singh
Sachin Singh

Reputation: 1098

Yes, the transfers_range query can be avoided by breaking your query into multiple lines and adding a condition on created_at presence separately. You can also combine both the OR queries into a single query like this:

def transfers(material, capacity, category, created_at)
  transfer_data = Transfer.where('sender_dispensary_id = ? OR receiver_dispensary_id = ?', id).where(status: %i[dispatched released returned], capacity: capacity, material_id: material)
  transfer_data = transfer_data.where(created_at: Time.given_month_range(created_at)) if created_at.present?
  transfer_data
end

Upvotes: 1

Related Questions