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.

\$\begingroup\$ What the url to #index will look like with tags? \$\endgroup\$

Billy Chan
– Billy Chan

2013-09-01 13:57:00 +00:00
Commented Sep 1, 2013 at 13:57 — Billy Chan
– Billy Chan, Commented Sep 1, 2013 at 13:57
\$\begingroup\$ Like so - /sites?tags=football+hockey \$\endgroup\$

Ashley
– Ashley

2013-09-01 14:27:26 +00:00
Commented Sep 1, 2013 at 14:27 — Ashley
– Ashley, Commented Sep 1, 2013 at 14:27

Ashley · Accepted Answer · 2013-08-31 16:34:50Z

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.

Billy Chan · Accepted Answer · 2013-09-01 15:47:19Z

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.

Stack Exchange Network

Rails 3 nested controller method

2 Answers 2

You must log in to answer this question.

Hot Network Questions

Rails 3 nested controller method

2 Answers 2

You must log in to answer this question.

Related

Hot Network Questions