Redmine Refactor #89: Extract Controller: IssueMovesController

With #move and #perform_move separated in the IssuesController, I can now see a clear path to using extract controller to move them to a new controller. (Extract controller is a Rails specific refactoring that is based on extract class).

Before

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27

class IssuesController < ApplicationController
  def move
    # ...
  end
 
  def perform_move
    # ...
  end
 
  private
 
  def prepare_for_issue_move
    # ...
  end
 
  def find_issues
    # ..
  end
 
  def set_flash_from_bulk_issue_save
    # ...
  end
 
  def extract_changed_attributes_for_move
    # ...
  end
end
 
1
2
3
4
5
6
7
8
9
10
11

# config/routes.rb
  map.with_options :controller => 'issues' do |issues_routes|
    issues_routes.with_options :conditions => {:method => :get} do |issues_views|
      # ...
      issues_views.connect 'issues/:id/move', :action => 'move', :id => /\d+/
    end
    issues_routes.with_options :conditions => {:method => :post} do |issues_actions|
      # ...
      issues_actions.connect 'issues/:id/:action', :action => /edit|perform_move|destroy/, :id => /\d+/
    end
  end

After

1
2
3

class IssuesController < ApplicationController
  # Methods removed
end
 
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31

class IssueMovesController < ApplicationController
  default_search_scope :issues
  before_filter :find_issues
  before_filter :authorize
 
  def new
    # ...
  end
 
  def create
    # ...
  end
 
  private
 
  def prepare_for_issue_move
    # ...
  end
 
  def find_issues
    # ..
  end
 
  def set_flash_from_bulk_issue_save
    # ...
  end
 
  def extract_changed_attributes_for_move
    # ...
  end
end
 
1
2

# config/routes.rb
  map.resources :issue_moves, :only => [:new, :create], :path_prefix => '/issues', :as => 'move'

The results of this refactoring are:

  • IssuesController had two actions removed.
  • A new RESTful resource was created, IssueMoves.
  • Removed some complex and nested route in favor of a map.resources route.
  • Added some code duplication in the utility methods of IssueMovesController.

Since the new REST resource was extracted, it wouldn’t be difficult to add a new API for moving and copying issues. But first I need to do a few more refactorings to remove the duplication I introduced in IssueMovesController.

Reference commit