Daily Refactor #55: Move Pane Configuration To KanbanPane

The Refactoring

I picked a big refactoring today. What started out as a small isolated change, grew and grew as I found dependencies all over the plugin. I used move method primarily but there are several other refactorings along for the ride.

Before you get hit with the wall of code, look for the code that’s calling pane_configured? and compare it before and after.

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

# app/helpers/kanbans_helper.rb
module KanbansHelper
  def pane_configured?(pane)
    (@settings['panes'] && @settings['panes'][pane] && !@settings['panes'][pane]['status'].blank?)
  end
 
  def display_pane?(pane)
    if pane == 'quick-tasks'
      pane_configured?('backlog') &&
        @settings['panes']['quick-tasks']['limit'].present? &&
        @settings['panes']['quick-tasks']['limit'].to_i > 0
    else
      pane_configured?(pane)
    end
  end
 
  def column_configured?(column)
    case column
    when :unstaffed
      pane_configured?('incoming') || pane_configured?('backlog')
    when :selected
      display_pane?('quick-tasks') || pane_configured?('selected')
    when :staffed
      true # always
    end
  end
 
  # Calculates the width of the column.  Max of 96 since they need
  # some extra for the borders.
  def staffed_column_width(column)
    # weights of the columns
    column_ratios = {
      :user => 1,
      :active => 2,
      :testing => 2,
      :finished => 2,
      :canceled => 2
    }
    return 0.0 if column == :active && !pane_configured?(:active)
    return 0.0 if column == :testing && !pane_configured?(:testing)
    return 0.0 if column == :finished && !pane_configured?(:finished)
    return 0.0 if column == :canceled && !pane_configured?(:canceled)
 
    visible = 0
    visible += column_ratios[:user]
    visible += column_ratios[:active] if pane_configured?(:active)
    visible += column_ratios[:testing] if pane_configured?(:testing)
    visible += column_ratios[:finished] if pane_configured?(:finished)
    visible += column_ratios[:canceled] if pane_configured?(:canceled)
 
    return ((column_ratios[column].to_f / visible) * 96).round(2)
  end
end
 
1
2
3
4

# app/models/kanban_pane.rb
class KanbanPane
  # ...
end
 
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20

# app/views/kanbans/show.html.erb
 
  <div id="unstaffed-requests" class="column" style="width: %">
 
    # ...
 
 
    # ...
 
  </div>
 
 
  &lt;div id=&quot;selected-requests&quot; class=&quot;column&quot; style=&quot;width: %"&gt;
 
    # ...
 
 
    # ...
 
  </div>

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

# app/helpers/kanbans_helper.rb
module KanbansHelper
  # pane_configured? removed
 
  # display_pane? removed
 
  def column_configured?(column)
    case column
    when :unstaffed
      KanbanPane::IncomingPane.configured? || KanbanPane::BacklogPane.configured?
    when :selected
      KanbanPane::QuickPane.configured? || KanbanPane::SelectedPane.configured?
    when :staffed
      true # always
    end
  end
 
  # Calculates the width of the column.  Max of 96 since they need
  # some extra for the borders.
  def staffed_column_width(column)
    # weights of the columns
    column_ratios = {
      :user =&gt; 1,
      :active =&gt; 2,
      :testing =&gt; 2,
      :finished =&gt; 2,
      :canceled =&gt; 2
    }
    return 0.0 if column == :active &amp;&amp; !KanbanPane::ActivePane.configured?
    return 0.0 if column == :testing &amp;&amp; !KanbanPane::TestingPane.configured?
    return 0.0 if column == :finished &amp;&amp; !KanbanPane::FinishedPane.configured?
    return 0.0 if column == :canceled &amp;&amp; !KanbanPane::CanceledPane.configured?
 
    visible = 0
    visible += column_ratios[:user]
    visible += column_ratios[:active] if KanbanPane::ActivePane.configured?
    visible += column_ratios[:testing] if KanbanPane::TestingPane.configured?
    visible += column_ratios[:finished] if KanbanPane::FinishedPane.configured?
    visible += column_ratios[:canceled] if KanbanPane::CanceledPane.configured?
 
    return ((column_ratios[column].to_f / visible) * 96).round(2)
  end
end
 
1
2
3
4
5
6
7
8
9
10
11

# app/models/kanban_pane.rb
class KanbanPane
  def self.pane_name
    self.to_s.demodulize.gsub(/pane/i, '').downcase
  end
 
  def self.configured?
    pane = self.pane_name
    (settings['panes'] &amp;&amp; settings['panes'][pane] &amp;&amp; !settings['panes'][pane]['status'].blank?)
  end
end
 
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22

# app/views/kanbans/show.html.erb
 
  &lt;div id=&quot;unstaffed-requests&quot; class=&quot;column&quot; style=&quot;width: %"&gt;
 
    <div class="pane">
       'incoming' %&gt;
    </div>
 
 
    # ...
 
  </div>
 
 
  &lt;div id=&quot;selected-requests&quot; class=&quot;column&quot; style=&quot;width: %"&gt;
 
    # ...
 
 
    # ...
 
  </div>

Review

There is a lot of code here but it all comes down to moving the KanbansHelper#pane_configured? into the KanbanPane class. The nice thing about it is that the KanbanPane#configured? doesn’t use a parameter anymore and just uses the class to check the configuration. This will make it easier to add more panes over time.

Reference commit (has a lot more code and test changes than shown here)