Rails conditional sorting based on params

The following code sorts the current users bid model items based on a param condition :bid_status. It starts to get messy when checking if the project space (:real_property_sale_project) :failed_date is .present? or .nil? to exclude certain items from the returned results. How can I improve this?

def index
  @bids = current_user.bids.select { |bid| bid.real_property_sale.try(:real_property_sale_project).try(:failed_date).nil? }

  if params[:bid_status] == 'in_progress'
    @bids = @bids.select { |bid| bid.real_property_sale.status == 'published' }     
  elsif params[:bid_status] == 'awarded'
    @bids = @bids.select { |bid| bid.status == 'accepted' && bid.real_property_sale.try(:real_property_sale_project).try(:failed_date).nil? }
  elsif params[:bid_status] == 'lost'
    @bids = @bids.select { |bid| bid.status == 'rejected' }
  elsif params[:bid_status] == 'history'
    @bids = current_user.bids.select { |bid| bid.status == 'accepted' && bid.real_property_sale.try(:real_property_sale_project).try(:failed_date).present? }
  end

  @bids = @bids.paginate(:page => params[:page], :per_page => 9)
end

Don't use select - that requires loading all records, and then filtering/sorting them in Ruby. But sorting and filtering is what databases and SQL is for.

For the first part, something like this should work (my Rails query-fu is rusty, so no guarantees):

current_user.bids
  .joins(real_property_sale: :real_property_sale_projects)
  .where(real_property_sale_project: { failed_date: nil })

On a side note, it seems like your models are overly complicated. They share the same prefix ("real property sale") which would seem to suggest they're been split up in a strange way. Not saying that's the case, but it does seem... off, somehow.

Whatever your app is doing, I'd wager that a user's bids are pretty important. So it seems strange that it'd take this much work to load them. A "bid status" should probably be front-and-center in the models; not a complex derived value, requiring loading not just 1st order but 2nd order associations. Just seems suspect to me.

Anyway, that code should probably be hidden away in a scope on the Bid model, so you can just say:

current_user.bids.active

or whatever name makes sense for this particular collection of bids.

Also, in your current code it seems that for the "awarded" value, you redo the same filtering you've already done once (checking failed_date).

Anyway, I imagine something like this:

@bids = case params[:bid_status]
        when 'in_progress'
          current_user.bids.active.where(real_property_sale: { status: "published" })
        when 'awarded'
          current_user.bids.active.where(status: "accepted")
        when 'lost'
          current_user.bids.active.where(status: "rejected")
        when 'history'
          current_user.bids
            .joins(real_property_sale: :real_property_sale_projects)
            .where("real_property_sale_project.failed_date IS NOT NULL")
        else
          current_user.bids.active
        end

Again, I can't guarantee it'll work as-is, but it illustrates the structure.

Note also that scopes can be chained, so you could conceivably make scopes for each of the filters, and do things like bids.active.accepted, bids.active.rejected, etc..

If you have named scopes for each filter, you could even do stuff like:

current_user.bids.active.send(params[:bid_status])

provided that a) you're sure it's safe to send the param (i.e. it's been whitelisted), and b) you use the same param values as the scope names.

Right now, it's a little odd that none of your param values actually match anything in the data models; "awarded" really means "accepted", "lost" really means "rejected", "in progress" actually means "published", etc..

Again, it seems like your data modelling is little off.

StackExchange Code Review Q#113224, answer score: 5