Filtering index using has_many through relationships

Question

Both Cities and Positions are associated to Jobs through a has_many :through relationship.

On the Jobs#Index page, I have a form, which when submitted allows a user to filter jobs based on a combination of city and positions.

I got this to work, but know the code can be a lot better.

This is my Jobs#Index controller:

 def index 
 @cities = City.all
 @positions = Position.all
 @city = City.find(params[:city_id]) if params[:city_id] && params[:city_id] != "0"
 @position = Position.find(params[:position_id]) if (params[:position_id] && params[:position_id] != "0")
 @jobs = Job.all
 @jobs = @jobs.includes(:cities).where(cities: { id: @city }) if @city
 @jobs = @jobs.includes(:positions).where(positions: { id: @position }) if @position
 @jobs = @jobs.paginate(page: params[:page], per_page: 5)
 end

This is my Jobs#Index View:

- provide(:title, 'All Jobs')
.thinstripe_career
 .darken
 .container
 .row
 .span10.offset1
 .jobs_header
 .row
 .span7
 h2 All #{@city.name if @city} #{@position.name if @position} Jobs
 .span3
 == link_to "Sign up for Job Alerts!", "#", :class => "button"
- if current_user && current_user.admin? 
 == render 'shared/dashboard_header' 
.container
 .row
 .span10.offset1
 .job_search
 == form_tag jobs_path, :method => 'get', :id => "jobs_filter" do
 h2 I'd like jobs in
 .filter_sect
 == select_tag :city_id, content_tag(:option,'all cities...', :value=>"0") + options_from_collection_for_select(@cities, "id", "name", params[:city_id].to_i ), :class => "basic" 
 .filter_sect
 == select_tag :position_id, content_tag(:option,'all roles...',:value=>"0") + options_from_collection_for_select(@positions, "id", "name", params[:position_id].to_i ), :class => "basic" 
 .filter_sect.filter_search
 == submit_tag "Search", :class => "button search_button button_black", :name => nil
 ul.jobs
 - if @jobs.any?
 == render @jobs
 - else
 li.non_founds
 h2 No jobs founds.
 p There are currently no jobs founds for these combinations. Sign up to requests
 == will_paginate

Answer 1

In models/job.rb, you can add scopes and use them chained on the controller:

class Job < ActiveRecord::Model
 scope :with_cities, ->(city) { includes(:cities).where(cities: { id: @city }) if city }
 scope :with_positions, ->(position) { includes(:positions).where(positions: { id: @position }) if position }
 # rest of the code ...
end

---

For JobsController, you can:

• Add a before_filter to load all the data, and if you need to use that data on other actions, you just need to append the name of that action too.

• Use memoization to load the city and the position.

In controllers/jobs_controller.rb:

class JobsController < ApplicationController
 before_filter :load_data, only: [:index]
 def index
 @jobs = Job.with_cities(city).with_positions(position).paginate(page: params[:page], per_page: 5)
 end
 # ...
 protected:
 def load_data
 @cities = City.all
 @positions = Position.all
 end
 def city
 @city ||= City.find(params[:city_id]) unless params[:city_id].empty?
 end
 def position
 @position ||= Position.find(params[:position_id]) unless params[:position_id].empty?
 end

---

On the view:

Use try, so instead of:

 - if current_user && current_user.admin? 
 == render 'shared/dashboard_header' 

you could write:

 == render 'shared/dashboard_header' if current_user.try(:admin?)

Use include_blank and prompt options in select_tag. This not only simplifies the views, but also allows to use params[city_id].empty? on the controller.

select_tag :city_id, 
 options_from_collection_for_select(@cities, :id, :name, params[:city_id].to_i), 
 include_blank: true, prompt: "all cities...", class: "basic" 

Finally, you could extract ul.jobs into a partial:

= render partial: 'jobs', locals: { jobs: @jobs }

Or even the .job_search div, especially if you'll be needing this somewhere else.