Redmine Refactor #90: Pull Up Method to ApplicationController

Now, in order to finish up IssueMovesController, I need to refactor the duplicated methods that I copied from IssuesController. Pull up method is a great refactoring to use in this case.

Before

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

class IssuesController < ApplicationController
  # Filter for bulk operations
  def find_issues
    @issues = Issue.find_all_by_id(params[:id] || params[:ids])
    raise ActiveRecord::RecordNotFound if @issues.empty?
    projects = @issues.collect(&:project).compact.uniq
    if projects.size == 1
      @project = projects.first
    else
      # TODO: let users bulk edit/move/destroy issues from different projects
      render_error 'Can not bulk edit/move/destroy issues from different projects'
      return false
    end
  rescue ActiveRecord::RecordNotFound
    render_404
  end
end
 
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18

class IssueMovesController < ApplicationController
  # Filter for bulk operations
  # TODO: duplicated in IssuesController
  def find_issues
    @issues = Issue.find_all_by_id(params[:id] || params[:ids])
    raise ActiveRecord::RecordNotFound if @issues.empty?
    projects = @issues.collect(&:project).compact.uniq
    if projects.size == 1
      @project = projects.first
    else
      # TODO: let users bulk edit/move/destroy issues from different projects
      render_error 'Can not bulk edit/move/destroy issues from different projects'
      return false
    end
  rescue ActiveRecord::RecordNotFound
    render_404
  end
end
 
1
2
3

class ApplicationController < ActionController::Base
  # ...
end

After

1
2
3

class IssuesController < ApplicationController
  # ...
end
 
1
2
3

class IssueMovesController < ApplicationController
  # ...
end
 
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17

class ApplicationController < ActionController::Base
  # Filter for bulk issue operations
  def find_issues
    @issues = Issue.find_all_by_id(params[:id] || params[:ids])
    raise ActiveRecord::RecordNotFound if @issues.empty?
    projects = @issues.collect(&:project).compact.uniq
    if projects.size == 1
      @project = projects.first
    else
      # TODO: let users bulk edit/move/destroy issues from different projects
      render_error 'Can not bulk edit/move/destroy issues from different projects'
      return false
    end
  rescue ActiveRecord::RecordNotFound
    render_404
  end
end

Now that this method has been moved, there is only one more duplicate method in IssueMovesController, set_flash_from_bulk_issue_save. I’ll tackle this refactoring tomorrow.

Reference commit