records.rb

# How to refactor?

def index
  @records = Record.includes(:category, :campus, :event).order("records.id ASC").page(params[:page]).per(params[:per_page])
  @records = @records.where(params.slice(:campus_id, :event_id, :category_id, :week_reference))
  @records = @records.where("service_date_time >= ?", params[:start_time]) if params[:start_time]
  @records = @records.where("service_date_time <= ?", params[:end_time]) if params[:end_time]
  @records = @records.where("week_reference >= ?", params[:start_week]) if params[:start_week]
  @records = @records.where("week_reference <= ?", params[:end_week]) if params[:end_week]
end

dude, move these to scopes

class Record < ActiveRecord::Base

  scope :after_week,             ->(week){ where("week_reference >= #{week}"  } 
  scope :before_week,          ->(week){ where("week_reference <= #{week}"  } 
  scope :after_service_time, ->(date_time){ where("service_date_time >= #{date_time}"  } 
  scope :before_service_time,  ->(date_time){ where("service_date_time <= #{date_time}"  } 
  scope :includes_for_index     -> { includes(:category, :campus, :event) } 

end

#controller
   #no need to order by id ASC, Active Record will do that by default
  @records = Records.includes_for_index.page(params[:page]).per(params[:per_page])
  @records = @records.after_week(params[:start_week]) if params[:start_week]  @records = @records.after_week(params[:start_week]) if params[:start_week]
....

@dylanjha Thanks!

Totally agree on the scopes. Next step is to figure out how to remove the if checks on each line...

Dat skinny controller.

class RecordsController
  def index
    Record.filter_by_params(record_filter_params)
  end

protected

  def record_filter_params
    params.slice(
      :campus_id, :event_id, :category_id, :week_reference,
      :start_time, :end_time, :start_week, :end_week
    )
  end
end

Naive refactor into a fat model method:

class Record
  def self.filter_by_params(params)
    records = where(params.slice(:campus_id, :event_id, :category_id, :week_reference))
    records = records.where("service_date_time >= ?", params[:start_time]) if params[:start_time]
    records = records.where("service_date_time <= ?", params[:end_time]) if params[:end_time]
    records = records.where("week_reference >= ?", params[:start_week]) if params[:start_week]
    records = records.where("week_reference <= ?", params[:end_week]) if params[:end_week]
    records
  end
end

+1 on semantic scopes, which are a good place to inline the conditionals:

class Record
  scope :after_week,          ->(week=nil){ week ? where("week_reference >= ?", week) : scoped }
  scope :before_week,         ->(week=nil){ week ? where("week_reference <= ?", week) : scoped } 
  scope :after_service_time,  ->(time=nil){ time ? where("service_date_time >= ?", time) : scoped } 
  scope :before_service_time, ->(time=nil){ time ? where("service_date_time <= ?", time) : scoped } 

  def self.filter_by_params(params)
    where(params.slice(:campus_id, :event_id, :category_id, :week_reference)).
      after_time(params[:start_time]).
      before_time(params[:end_time]).
      after_week(params[:start_week]).
      before_week(params[:end_week])
  end
end

Also, re: @dylanjha's scope definitions — does ActiveRecord sanitize Ruby interpolation now and I missed it?

Thanks, @nz

I just wrote something very similar. To reduce all the ternary's, I'm calling this first:

  def self.scope_if_value(scope, value)
    return value.present? ? Record.send(scope, value) : scoped
  end

+1 on filter_by_params -- gonna use that to slim down the controller.