Rails 3 nested controller method

Question

I have 2 nested resources in one of my Rails apps and the way the index is handled by the inner resources bothers me somewhat:

def index
    @program = Program.approved.find(params[:program_id]) if params[:program_id]
    if params[:tags]
      tags = params[:tags].split(" ")
      if @program
        @sites = Site.where(:program_id => @program.id).tagged_with(tags).page(params[:page])
      else
        @sites = Site.joins(:program => :user).tagged_with(tags).page(params[:page])
      end
    else
      if @program
        @sites = Site.where(:program_id => @program.id).page(params[:page])
      else
        @sites = Site.joins(:program => :user).page(params[:page])
      end
    end

    respond_to do |format|
      format.html # index.html.erb
      format.json { render json: @sites }
    end
end

The complexity is caused because I want my Site resource to be accessible both through it's Program or directly. The results are also optionally filtered based on tags if they are included in the params.

The joins are required because if a site belongs to a user then I display Edit/Delete options.

It just doesn't feel like good Ruby code. So any refactoring tips would be helpful.

Answer 1

You could get rid of the code duplication by making use of the fact, that arel expressions are lazily evaluated:

def index
  @program = Program.approved.find(params[:program_id]) if params[:program_id]

  sites =
    if @program
      Site.where(program_id: @program)
    else
      Site.joins(program: :user)
    end
  if params[:tags]
    tags = params[:tags].split(" ")
    sites = sites.tagged_with(tags)
  end
  @sites = sites.page(params[:page])

  respond_to do |format|
    format.html
    format.json { render json: @sites }
  end
end

Because you're using ruby 1.9 hash syntax in the render expression I also adapted the other hashes. That seems more consistent for me.

Answer 2

A better place for such logic is the model, to make things simpler and cleaner.

#SitesController
def index
  @sites = Site.scope_filtered(params)
  respond_to do |format|
    format.html # index.html.erb
    format.json { render json: @sites }
  end
end

#Site model
def self.scope_filtered(args)
  result = args[:program_id] ? where(program_id: args[:program_id]) : joins(program: :user)
  result = result.tagged_with(args[:tags].split(' ')) if args[:tags]
  result = result.page(args[:page]) if args[:page]
end

Side note: I'm not very sure what :user is in joins(:program =>:user). I assume it will work according to original code.