Daily Refactor #15: Rename method in IssueStatusesController

It’s time to move on from the ReportsController to code that needs to be refactored more.

Using Caliper’s Flay Report, it looks like IssueStatusesController, TrackersController, AuthSourcesController, and RolesController all have the exact same index action.

1
2
3
4

  def index
    list
    render :action => 'list' unless request.xhr?
  end

While I could just use pull up method to remove this duplication, there is a stronger smell here that I’d like to fix…

The Refactoring

In Rails the index action is generally used to list a collection of records, typically with some pagination. These four controllers are using the list action for that though and have the index action just run list. This is way too much redirection and it can be simplified.

Before

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16

# app/controllers/issue_statuses_controller.rb
class IssueStatusesController  :post, :only => [ :destroy, :create, :update, :move, :update_issue_done_ratio ],
         :redirect_to => { :action => :list }
 
  def index
    list
    render :action => 'list' unless request.xhr?
  end
 
  def list
    @issue_status_pages, @issue_statuses = paginate :issue_statuses, :per_page => 25, :order => "position"
    render :action => "list", :layout => false if request.xhr?
  end
 
  # ...
end

After

1
2
3
4
5
6
7
8
9
10
11

# app/controllers/issue_statuses_controller.rb
class IssueStatusesController  :post, :only => [ :destroy, :create, :update, :move, :update_issue_done_ratio ],
         :redirect_to => { :action => :index }
 
  def index
    @issue_status_pages, @issue_statuses = paginate :issue_statuses, :per_page => 25, :order => "position"
    render :action => "index", :layout => false if request.xhr?
  end
 
  # ...
end

Review

Now IssueStatusesController only has the index action, which follows the Rails standards. This will also make it easier to add on a RESTful web service later on.

Reference commit (includes the changes to the tests and redirects that used list)