Extract pie chart data logic into model layer

Move most of the tests for this logic into the unit test layer.

app/controllers/stats_controller.rb

@@ -178,20 +178,18 @@ class StatsController < ApplicationController
   end
 
   def context_total_actions_data
-    all_actions_per_context = Stats::TopContextsQuery.new(current_user).result
+    actions = Stats::TopContextsQuery.new(current_user).result
-    @title = t('stats.spread_of_actions_for_all_context')
+
-    @alpha = 70
+    @data = Stats::PieChartData.new(actions, t('stats.spread_of_actions_for_all_context'), 70)
-    prep_context_data_for_view(all_actions_per_context)
+    @data.calculate
 
     render :pie_chart_data, :layout => false
   end
 
   def context_running_actions_data
-    all_actions_per_context = Stats::TopContextsQuery.new(current_user, :running => true).result
+    actions = Stats::TopContextsQuery.new(current_user, :running => true).result
-
+    @data = Stats::PieChartData.new(actions, t('stats.spread_of_running_actions_for_visible_contexts'), 60)
-    @title = t('stats.spread_of_running_actions_for_visible_contexts')
+    @data.calculate
-    @alpha = 60
-    prep_context_data_for_view(all_actions_per_context)
 
     render :pie_chart_data, :layout => false
   end
@@ -345,31 +343,6 @@ class StatsController < ApplicationController
 
   private
 
-  def prep_context_data_for_view(all_actions_per_context)
-
-    sum = all_actions_per_context.inject(0){|sum, apc| sum + apc['total']}
-
-    pie_cutoff=10
-    size = [all_actions_per_context.size, pie_cutoff].min
-
-    # explicitly copy contents of hash to avoid ending up with two arrays pointing to same hashes
-    actions_per_context = Array.new(size){|i| {
-      'name' => all_actions_per_context[i]['name'],
-      'total' => all_actions_per_context[i]['total'],
-      'id' => all_actions_per_context[i]['id']
-    } }
-
-    if all_actions_per_context.size > pie_cutoff
-      actions_per_context[-1]['name']=t('stats.other_actions_label')
-      actions_per_context[-1]['id']=-1
-      size.upto(all_actions_per_context.size-1){ |i| actions_per_context[-1]['total']+=(all_actions_per_context[i]['total']) }
-    end
-
-    @pie_slices = Array.new(size){|i| actions_per_context[i]['total']*100/sum }
-    @pie_labels = Array.new(size){|i| actions_per_context[i]['name'].truncate(15, :omission => '...') }
-    @pie_links  = Array.new(size){|i| context_path(actions_per_context[i]['id'])}
-  end
-
   def init
     @me = self # for meta programming
 

app/models/stats/pie_chart_data.rb

@@ -0,0 +1,37 @@
+module Stats
+  class PieChartData
+
+    attr_reader :all_actions_per_context, :values, :labels, :ids, :alpha, :title
+    def initialize(all_actions_per_context, title, alpha)
+      @all_actions_per_context = all_actions_per_context
+      @title = title
+      @alpha = alpha
+    end
+
+    def calculate
+      sum = all_actions_per_context.inject(0){|sum, apc| sum + apc['total']}
+
+      pie_cutoff=10
+      size = [all_actions_per_context.size, pie_cutoff].min
+
+      # explicitly copy contents of hash to avoid ending up with two arrays pointing to same hashes
+      actions_per_context = Array.new(size){|i| {
+        'name' => all_actions_per_context[i]['name'],
+        'total' => all_actions_per_context[i]['total'],
+        'id' => all_actions_per_context[i]['id']
+      } }
+
+      if all_actions_per_context.size > pie_cutoff
+        actions_per_context[-1]['name']=I18n.t('stats.other_actions_label')
+        actions_per_context[-1]['id']=-1
+        size.upto(all_actions_per_context.size-1){ |i| actions_per_context[-1]['total']+=(all_actions_per_context[i]['total']) }
+      end
+
+      @values = Array.new(size){|i| actions_per_context[i]['total']*100/sum }
+      @labels = Array.new(size){|i| actions_per_context[i]['name'].truncate(15, :omission => '...') }
+      @ids = Array.new(size){|i| actions_per_context[i]['id']}
+    end
+
+
+  end
+end

app/views/stats/pie_chart_data.html.erb

@@ -1,9 +1,9 @@
-&title=<%= @title %>,{font-size:16}&
+&title=<%= @data.title %>,{font-size:16}&
-&pie=<%= @alpha %>,#505050,{font-size: 12px; color: #404040;}&
+&pie=<%= @data.alpha %>,#505050,{font-size: 12px; color: #404040;}&
 &x_axis_steps=1& &y_ticks=5,10,5& &line=3,#87421F& &y_min=0& &y_max=20&
-&values=<%=     @pie_slices.join(",") %>&
+&values=<%=     @data.values.join(",") %>&
-&pie_labels=<%= @pie_labels.join(",") %>&
+&pie_labels=<%= @data.labels.join(",") %>&
-&links=<%=      @pie_links.join(",") %>&
+&links=<%=      @data.ids.map{|id| context_path(id)}.join(",") %>&
 &colours=#d01f3c,#356aa0,#C79810,#c61fd0,#1fc6d0,#1fd076,#72d01f,#c6d01f,#d0941f,#40941f&
 &tool_tip=#x_label#: #val#%25&
 &x_label_style=9,,2,1&

test/functional/context_actions_data_test.rb

@@ -3,152 +3,32 @@ require File.expand_path(File.dirname(__FILE__) + '/../test_helper')
 class ContextActionsDataTest < ActionController::TestCase
   tests StatsController
 
-  def test_total_with_0_items
-    login_as(:admin_user)
-    Stats::TopContextsQuery.any_instance.stubs(:result).returns []
-
-    get :context_total_actions_data
-
-    assert_equal [], assigns[:actions_per_context]
-  end
-
-  def test_total_with_less_than_10_items
-    login_as(:admin_user)
-    contexts = [
-      {'id' => 1, 'name' => 'one', 'total' => 11},
-      {'id' => 2, 'name' => 'two', 'total' => 4},
-      {'id' => 3, 'name' => 'three', 'total' => 8},
-      {'id' => 4, 'name' => 'four', 'total' => 13},
-      {'id' => 5, 'name' => 'five', 'total' => 20},
-      {'id' => 6, 'name' => 'six', 'total' => 17},
-      {'id' => 7, 'name' => 'seven', 'total' => 5},
-      {'id' => 8, 'name' => 'eight', 'total' => 1},
-      {'id' => 9, 'name' => 'nine', 'total' => 6}
-    ]
-    Stats::TopContextsQuery.any_instance.stubs(:result).returns contexts
-
-    get :context_total_actions_data
-
-    assert_equal contexts, assigns[:actions_per_context]
-  end
-
-  def test_total_with_exactly_10_items
-    login_as(:admin_user)
-    contexts = [
-      {'id' => 1, 'name' => 'one', 'total' => 11},
-      {'id' => 2, 'name' => 'two', 'total' => 4},
-      {'id' => 3, 'name' => 'three', 'total' => 8},
-      {'id' => 4, 'name' => 'four', 'total' => 13},
-      {'id' => 5, 'name' => 'five', 'total' => 20},
-      {'id' => 6, 'name' => 'six', 'total' => 17},
-      {'id' => 7, 'name' => 'seven', 'total' => 5},
-      {'id' => 8, 'name' => 'eight', 'total' => 1},
-      {'id' => 9, 'name' => 'nine', 'total' => 6},
-      {'id' => 10, 'name' => 'ten', 'total' => 19}
-    ]
-    Stats::TopContextsQuery.any_instance.stubs(:result).returns contexts
-
-    get :context_total_actions_data
-
-    assert_equal contexts, assigns[:actions_per_context]
-  end
-
   def test_total_with_more_than_10_items
     login_as(:admin_user)
     contexts = [
       {'id' => 1, 'name' => 'one', 'total' => 11},
       {'id' => 2, 'name' => 'two', 'total' => 4},
-      {'id' => 3, 'name' => 'three', 'total' => 8},
+      {'id' => 3, 'name' => 'three', 'total' => 8}
-      {'id' => 4, 'name' => 'four', 'total' => 13},
-      {'id' => 5, 'name' => 'five', 'total' => 20},
-      {'id' => 6, 'name' => 'six', 'total' => 17},
-      {'id' => 7, 'name' => 'seven', 'total' => 5},
-      {'id' => 8, 'name' => 'eight', 'total' => 1},
-      {'id' => 9, 'name' => 'nine', 'total' => 6},
-      {'id' => 10, 'name' => 'ten', 'total' => 19},
-      {'id' => 11, 'name' => 'eleven', 'total' => 14}
     ]
     Stats::TopContextsQuery.any_instance.stubs(:result).returns contexts
 
     get :context_total_actions_data
 
-    contexts.pop
+    assert_equal [47, 17, 34], assigns[:data].values
-    contexts[-1] = {'id' => -1, 'name' => '(others)', 'total' => 33}
-    assert_equal contexts, assigns[:actions_per_context]
   end
 
-  def test_running_with_0_items
+  def test_running_actions
-    login_as(:admin_user)
-    Stats::TopContextsQuery.any_instance.stubs(:result).returns []
-
-    get :context_running_actions_data
-
-    assert_equal [], assigns[:actions_per_context]
-  end
-
-  def test_running_with_less_than_10_items
     login_as(:admin_user)
     contexts = [
       {'id' => 1, 'name' => 'one', 'total' => 11},
       {'id' => 2, 'name' => 'two', 'total' => 4},
-      {'id' => 3, 'name' => 'three', 'total' => 8},
+      {'id' => 3, 'name' => 'three', 'total' => 8}
-      {'id' => 4, 'name' => 'four', 'total' => 13},
-      {'id' => 5, 'name' => 'five', 'total' => 20},
-      {'id' => 6, 'name' => 'six', 'total' => 17},
-      {'id' => 7, 'name' => 'seven', 'total' => 5},
-      {'id' => 8, 'name' => 'eight', 'total' => 1},
-      {'id' => 9, 'name' => 'nine', 'total' => 6}
     ]
     Stats::TopContextsQuery.any_instance.stubs(:result).returns contexts
 
     get :context_running_actions_data
 
-    assert_equal contexts, assigns[:actions_per_context]
+    assert_equal [47, 17, 34], assigns[:data].values
-  end
-
-  def test_running_with_exactly_10_items
-    login_as(:admin_user)
-    contexts = [
-      {'id' => 1, 'name' => 'one', 'total' => 11},
-      {'id' => 2, 'name' => 'two', 'total' => 4},
-      {'id' => 3, 'name' => 'three', 'total' => 8},
-      {'id' => 4, 'name' => 'four', 'total' => 13},
-      {'id' => 5, 'name' => 'five', 'total' => 20},
-      {'id' => 6, 'name' => 'six', 'total' => 17},
-      {'id' => 7, 'name' => 'seven', 'total' => 5},
-      {'id' => 8, 'name' => 'eight', 'total' => 1},
-      {'id' => 9, 'name' => 'nine', 'total' => 6},
-      {'id' => 10, 'name' => 'ten', 'total' => 19}
-    ]
-    Stats::TopContextsQuery.any_instance.stubs(:result).returns contexts
-
-    get :context_running_actions_data
-
-    assert_equal contexts, assigns[:actions_per_context]
-  end
-
-  def test_running_with_more_than_10_items
-    login_as(:admin_user)
-    contexts = [
-      {'id' => 1, 'name' => 'one', 'total' => 11},
-      {'id' => 2, 'name' => 'two', 'total' => 4},
-      {'id' => 3, 'name' => 'three', 'total' => 8},
-      {'id' => 4, 'name' => 'four', 'total' => 13},
-      {'id' => 5, 'name' => 'five', 'total' => 20},
-      {'id' => 6, 'name' => 'six', 'total' => 17},
-      {'id' => 7, 'name' => 'seven', 'total' => 5},
-      {'id' => 8, 'name' => 'eight', 'total' => 1},
-      {'id' => 9, 'name' => 'nine', 'total' => 6},
-      {'id' => 10, 'name' => 'ten', 'total' => 19},
-      {'id' => 11, 'name' => 'eleven', 'total' => 14}
-    ]
-    Stats::TopContextsQuery.any_instance.stubs(:result).returns contexts
-
-    get :context_running_actions_data
-
-    contexts.pop
-    contexts[-1] = {'id' => -1, 'name' => '(others)', 'total' => 33}
-    assert_equal contexts, assigns[:actions_per_context]
   end
 
 end

test/unit/pie_chart_data_test.rb

@@ -0,0 +1,88 @@
+require File.expand_path(File.dirname(__FILE__) + '/../minimal_test_helper')
+require 'app/models/stats/pie_chart_data'
+require 'active_support/core_ext/string'
+
+class Stats::PieChartDataTest < Test::Unit::TestCase
+
+  def setup
+    xx = { :stats => { :other_actions_label => '(other)' } }
+    I18n.backend.store_translations(:xx, xx)
+    I18n.locale = :xx
+  end
+
+  def test_with_0_items
+    data = Stats::PieChartData.new([], 'a chart', 50)
+    data.calculate
+
+    assert_equal [], data.values
+    assert_equal [], data.labels
+    assert_equal [], data.ids
+  end
+
+  def test_with_less_than_10_items
+    items = [
+      {'id' => 1, 'name' => 'one', 'total' => 11},
+      {'id' => 2, 'name' => 'two', 'total' => 4},
+      {'id' => 3, 'name' => 'three', 'total' => 8},
+      {'id' => 4, 'name' => 'four', 'total' => 13},
+      {'id' => 5, 'name' => 'five', 'total' => 20},
+      {'id' => 6, 'name' => 'six', 'total' => 17},
+      {'id' => 7, 'name' => 'seven', 'total' => 5},
+      {'id' => 8, 'name' => 'eight', 'total' => 1},
+      {'id' => 9, 'name' => 'nine', 'total' => 6}
+    ]
+
+    data = Stats::PieChartData.new(items, 'a chart', 50)
+    data.calculate
+
+    assert_equal [12, 4, 9, 15, 23, 20, 5, 1, 7], data.values
+    assert_equal ["one", "two", "three", "four", "five", "six", "seven", "eight", "nine"], data.labels
+    assert_equal [1, 2, 3, 4, 5, 6, 7, 8, 9], data.ids
+  end
+
+  def test_with_exactly_10_items
+    items = [
+      {'id' => 1, 'name' => 'one', 'total' => 11},
+      {'id' => 2, 'name' => 'two', 'total' => 4},
+      {'id' => 3, 'name' => 'three', 'total' => 8},
+      {'id' => 4, 'name' => 'four', 'total' => 13},
+      {'id' => 5, 'name' => 'five', 'total' => 20},
+      {'id' => 6, 'name' => 'six', 'total' => 17},
+      {'id' => 7, 'name' => 'seven', 'total' => 5},
+      {'id' => 8, 'name' => 'eight', 'total' => 1},
+      {'id' => 9, 'name' => 'nine', 'total' => 6},
+      {'id' => 10, 'name' => 'ten', 'total' => 19}
+    ]
+
+    data = Stats::PieChartData.new(items, 'a chart', 50)
+    data.calculate
+
+    assert_equal [10, 3, 7, 12, 19, 16, 4, 0, 5, 18], data.values
+    assert_equal ["one", "two", "three", "four", "five", "six", "seven", "eight", "nine", "ten"], data.labels
+    assert_equal [1, 2, 3, 4, 5, 6, 7, 8, 9, 10], data.ids
+  end
+
+  def test_with_more_than_10_items
+    items = [
+      {'id' => 1, 'name' => 'one', 'total' => 11},
+      {'id' => 2, 'name' => 'two', 'total' => 4},
+      {'id' => 3, 'name' => 'three', 'total' => 8},
+      {'id' => 4, 'name' => 'four', 'total' => 13},
+      {'id' => 5, 'name' => 'five', 'total' => 20},
+      {'id' => 6, 'name' => 'six', 'total' => 17},
+      {'id' => 7, 'name' => 'seven', 'total' => 5},
+      {'id' => 8, 'name' => 'eight', 'total' => 1},
+      {'id' => 9, 'name' => 'nine', 'total' => 6},
+      {'id' => 10, 'name' => 'ten', 'total' => 19},
+      {'id' => 11, 'name' => 'eleven', 'total' => 14}
+    ]
+
+    data = Stats::PieChartData.new(items, 'a chart', 50)
+    data.calculate
+
+    assert_equal [9, 3, 6, 11, 16, 14, 4, 0, 5, 27], data.values
+    assert_equal ["one", "two", "three", "four", "five", "six", "seven", "eight", "nine", "(other)"], data.labels
+    assert_equal [1, 2, 3, 4, 5, 6, 7, 8, 9, -1], data.ids
+  end
+
+end