5
\$\begingroup\$

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
\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

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.

\$\endgroup\$
0

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.