Rails: Filter based on multiple params?

Question

I have a simple app and the user can filter based on three different params by category, by discipline and by target group. All params are optional. So far it works, but the controller below is a bit bloated and I also think the three if statements are a bit messy.

Is there a cleaner way to do this?

def index
@listings = Listing.includes(:categorizations, :listing_disciplines, :listing_targets).page(params[:page])

if params[:category_id].present? && params[:category_id] != ""
  @category_id = Category.find_by(id: params[:category_id])
  @listings = @category_id.listings
end

if params[:discipline_id].present? && params[:discipline_id] != ""
  @discipline_id = Discipline.find_by(id: params[:discipline_id])
  @listings = @discipline_id.listings
end

if params[:target_id].present? && params[:target_id] != ""
  @target_id = Target.find_by(id: params[:target_id])
  @listings = @target_id.listings 
end

if params.blank?
  @listings   
end

And here's my listing model:

class Listing < ActiveRecord::Base
  has_many :categorizations, dependent: :destroy
  has_many :categories, through: :categorizations

  has_many :listing_disciplines, dependent: :destroy
  has_many :disciplines, through: :listing_disciplines

  has_many :listing_targets, dependent: :destroy
  has_many :targets, through: :listing_targets

  has_attached_file :image, styles: { :medium => "200x200#" }
  validates_attachment_content_type :image, content_type: /\Aimage\/.*\Z/

  validates :title, presence: true, uniqueness: true

  paginates_per 15

end

Fat models and skinny controllers. The Listing domain class should be the one to implement the filtering based on data it receives from the controller (or whatever entity makes a request for a set of listings). Just my $0.02. — railsdog
– railsdog, Commented Jan 3, 2015 at 15:36
Thanks @railsdog, I will try to refactor and implement the filtering into the listing model instead to keep the controller skinny. — CKP
– CKP, Commented Jan 3, 2015 at 16:32

ptd · Accepted Answer · 2015-01-03 17:10:37Z

1

I'm not confident that your code even works, because you are resetting listings with each param. I assume you want to be able to filter by all three at once. A cleaner way would be:

def index
  @listings = Listing.joins(:categorizations, :listing_disciplines, :listing_targets)
  @listings = @listings.where("categorizations.category_id = ?", params[:category_id]) if params[:category_id].present? && params[:category_id] != ""
  @listings = @listings.where("listings_disciplines.discipline_id = ?", params[:discipline_id]) if params[:discipline_id].present? && params[:discipline_id] != ""
  @listings = @listings.where("listing_targets.target_id = ?", params[:target_id]) if params[:target_id].present? && params[:target_id] != ""
  @listings = @listings.page(params[:page])
end

This will make all filters cumulative.

edited Jan 3, 2015 at 17:10

answered Jan 3, 2015 at 15:35

ptd

3,05418 silver badges30 bronze badges

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

1 Comment

house9 Over a year ago

The if params[:category_id].present? && params[:category_id] != "" can be shortened to if params[:category_id].present?; present returns false if the value is nil or a blank string - present returns the opposite of blank? - api.rubyonrails.org/classes/Object.html#method-i-blank-3F

Collectives™ on Stack Overflow

Rails: Filter based on multiple params?

1 Answer 1

1 Comment

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

1 Comment

Your Answer

Sign up or log in

Post as a guest

Linked

Related