Redmine Refactor #102: Move method from ProjectsController#roadmap to VersionsController#index

The next refactoring I performed on Redmine’s ProjectsController was to move the #roadmap method to VersionsController. The #roadmap is used to list all Versions on a project. Since that fits Rails’ RESTful conventions for #index, I used move method to move it to the VersionsController.

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
28
29
30
31
32
33
34

class ProjectsController  'position')
    retrieve_selected_tracker_ids(@trackers, @trackers.select {|t| t.is_in_roadmap?})
    @with_subprojects = params[:with_subprojects].nil? ? Setting.display_subprojects_issues? : (params[:with_subprojects] == '1')
    project_ids = @with_subprojects ? @project.self_and_descendants.collect(&:id) : [@project.id]
 
    @versions = @project.shared_versions || []
    @versions += @project.rolled_up_versions.visible if @with_subprojects
    @versions = @versions.uniq.sort
    @versions.reject! {|version| version.closed? || version.completed? } unless params[:completed]
 
    @issues_by_version = {}
    unless @selected_tracker_ids.empty?
      @versions.each do |version|
        issues = version.fixed_issues.visible.find(:all,
                                                   :include => [:project, :status, :tracker, :priority],
                                                   :conditions => {:tracker_id => @selected_tracker_ids, :project_id => project_ids},
                                                   :order => "#{Project.table_name}.lft, #{Tracker.table_name}.position, #{Issue.table_name}.id")
        @issues_by_version[version] = issues
      end
    end
    @versions.reject! {|version| !project_ids.include?(version.project_id) && @issues_by_version[version].blank?}
  end
 
  private
 
  def retrieve_selected_tracker_ids(selectable_trackers, default_trackers=nil)
    if ids = params[:tracker_ids]
      @selected_tracker_ids = (ids.is_a? Array) ? ids.collect { |id| id.to_i.to_s } : ids.split('/').collect { |id| id.to_i.to_s }
    else
      @selected_tracker_ids = (default_trackers || selectable_trackers).collect {|t| t.id.to_s }
    end
  end
 
end
 
1
2
3
4

class VersionsController  [:new, :close_completed]
  before_filter :find_project_from_association, :except => [:new, :close_completed]
  before_filter :find_project, :only => [:new, :close_completed]
end

After

1
2

class ProjectsController < ApplicationController
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
32
33
34
35
36
37
38
39

class VersionsController  [:index, :new, :close_completed]
  before_filter :find_project_from_association, :except => [:index, :new, :close_completed]
  before_filter :find_project, :only => [:index, :new, :close_completed]
 
  def index
    @trackers = @project.trackers.find(:all, :order => 'position')
    retrieve_selected_tracker_ids(@trackers, @trackers.select {|t| t.is_in_roadmap?})
    @with_subprojects = params[:with_subprojects].nil? ? Setting.display_subprojects_issues? : (params[:with_subprojects] == '1')
    project_ids = @with_subprojects ? @project.self_and_descendants.collect(&:id) : [@project.id]
 
    @versions = @project.shared_versions || []
    @versions += @project.rolled_up_versions.visible if @with_subprojects
    @versions = @versions.uniq.sort
    @versions.reject! {|version| version.closed? || version.completed? } unless params[:completed]
 
    @issues_by_version = {}
    unless @selected_tracker_ids.empty?
      @versions.each do |version|
        issues = version.fixed_issues.visible.find(:all,
                                                   :include => [:project, :status, :tracker, :priority],
                                                   :conditions => {:tracker_id => @selected_tracker_ids, :project_id => project_ids},
                                                   :order => "#{Project.table_name}.lft, #{Tracker.table_name}.position, #{Issue.table_name}.id")
        @issues_by_version[version] = issues
      end
    end
    @versions.reject! {|version| !project_ids.include?(version.project_id) && @issues_by_version[version].blank?}
  end
 
  private
 
  def retrieve_selected_tracker_ids(selectable_trackers, default_trackers=nil)
    if ids = params[:tracker_ids]
      @selected_tracker_ids = (ids.is_a? Array) ? ids.collect { |id| id.to_i.to_s } : ids.split('/').collect { |id| id.to_i.to_s }
    else
      @selected_tracker_ids = (default_trackers || selectable_trackers).collect {|t| t.id.to_s }
    end
  end
 
end

After I updated VersionsController‘s before_filters and moved the utility method, retrieve_selected_tracker_ids, it was easy to move #roadmap. I turned out lucky that the VersionsController didn’t try to use #index for anything already. Otherwise I wouldn’t have been able to do this refactoring and might have had to put #roadmap into it’s own controller.

Reference commit