Daily Refactor #47: Extract Method for Kanban panes – Part 3

Here is the third refactoring to clean up how the Kanban class gets it’s pane data.

The Refactoring

This refactoring is brutal to read through but it’s just moving the body of #get_incoming_issues, #get_backlog_issues, and #get_quick_issues into #get_issues_for_pane.

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
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55

# app/models/kanban.rb
class Kanban
  def get_incoming_issues
    return [[]] if missing_settings('incoming')
 
    return Issue.visible.find(:all,
                              :limit => @settings['panes']['incoming']['limit'],
                              :order => "#{Issue.table_name}.created_on ASC",
                              :conditions => {:status_id => @settings['panes']['incoming']['status']})
  end
 
  def get_backlog_issues(exclude_ids=[])
    return [[]] if missing_settings('backlog')
 
    conditions = ARCondition.new
    conditions.add ["#{Issue.table_name}.status_id IN (?)", @settings['panes']['backlog']['status']]
    conditions.add ["#{Issue.table_name}.id NOT IN (?)", exclude_ids] unless exclude_ids.empty?
 
    issues = Issue.visible.all(:limit => @settings['panes']['backlog']['limit'],
                               :order => "#{RedmineKanban::KanbanCompatibility::IssuePriority.klass.table_name}.position ASC, #{Issue.table_name}.created_on ASC",
                               :include => :priority,
                               :conditions => conditions.conditions)
 
    return group_by_priority_position(issues)
  end
 
  # TODO: similar to backlog issues
  def get_quick_issues
    return [[]] if missing_settings('quick-tasks', :skip_status => true) || missing_settings('backlog')
 
    issues = Issue.visible.all(:limit => @settings['panes']['quick-tasks']['limit'],
                               :order => "#{RedmineKanban::KanbanCompatibility::IssuePriority.klass.table_name}.position ASC, #{Issue.table_name}.created_on ASC",
                               :include => :priority,
                               :conditions => {:status_id => @settings['panes']['backlog']['status'], :estimated_hours => nil})
 
    return group_by_priority_position(issues)
  end
 
  private
 
  def get_issues_for_pane(pane)
    return [[]] unless [:finished, :canceled].include?(pane)
    return [[]] if missing_settings(pane.to_s)
 
    status_id = @settings['panes'][pane.to_s]['status']
    days = @settings['panes'][pane.to_s]['limit'] || 7
 
    issues = Issue.visible.all(:include => :assigned_to,
                               :order => "#{Issue.table_name}.updated_on DESC",
                               :conditions => ["#{Issue.table_name}.status_id = ? AND #{Issue.table_name}.updated_on > ?", status_id, days.to_f.days.ago])
 
    return issues.group_by(&:assigned_to)
  end
 
end

After

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
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71

# app/models/kanban.rb
class Kanban
  def get_incoming_issues
    get_issues_for_pane(:incoming)
  end
 
  def get_backlog_issues(exclude_ids=[])
    get_issues_for_pane(:backlog, :exclude_ids => exclude_ids)
  end
 
  def get_quick_issues
    get_issues_for_pane(:quick)
  end
 
  private
 
  def get_issues_for_pane(pane, options = {})
    case pane
    when :finished, :canceled
 
      return [[]] if missing_settings(pane.to_s)
 
      status_id = @settings['panes'][pane.to_s]['status']
      days = @settings['panes'][pane.to_s]['limit'] || 7
 
      issues = Issue.visible.all(:include => :assigned_to,
                                 :order => "#{Issue.table_name}.updated_on DESC",
                                 :conditions => ["#{Issue.table_name}.status_id = ? AND #{Issue.table_name}.updated_on > ?", status_id, days.to_f.days.ago])
 
      return issues.group_by(&:assigned_to)
 
    when :quick
      return [[]] if missing_settings('quick-tasks', :skip_status => true) || missing_settings('backlog')
 
      issues = Issue.visible.all(:limit => @settings['panes']['quick-tasks']['limit'],
                                 :order => "#{RedmineKanban::KanbanCompatibility::IssuePriority.klass.table_name}.position ASC, #{Issue.table_name}.created_on ASC",
                                 :include => :priority,
                                 :conditions => {:status_id => @settings['panes']['backlog']['status'], :estimated_hours => nil})
 
      return group_by_priority_position(issues)
 
    when :backlog
      return [[]] if missing_settings('backlog')
 
      exclude_ids = options.delete(:exclude_ids)
 
      conditions = ARCondition.new
      conditions.add ["#{Issue.table_name}.status_id IN (?)", @settings['panes']['backlog']['status']]
      conditions.add ["#{Issue.table_name}.id NOT IN (?)", exclude_ids] unless exclude_ids.empty?
 
      issues = Issue.visible.all(:limit => @settings['panes']['backlog']['limit'],
                                 :order => "#{RedmineKanban::KanbanCompatibility::IssuePriority.klass.table_name}.position ASC, #{Issue.table_name}.created_on ASC",
                                 :include => :priority,
                                 :conditions => conditions.conditions)
 
      return group_by_priority_position(issues)
 
    when :incoming
      return [[]] if missing_settings('incoming')
 
      return Issue.visible.find(:all,
                                :limit => @settings['panes']['incoming']['limit'],
                                :order => "#{Issue.table_name}.created_on ASC",
                                :conditions => {:status_id => @settings['panes']['incoming']['status']})
    else
      return [[]]
    end
 
  end
 
end

Review

Now that all of the extractions are done, all of the Kanban#get_* methods are using either #get_issues_for_panes or #issues_from_kanban_issue. Though this just shuffled statements around, it really cleared up how the public methods behave. Now I can dig into #get_issues_for_panes and #issues_from_kanban_issue and remove the duplication in them.

Reference commit