Factor out code for generating graphable data and add tests

This simplifies the statistics action of the PublicBodyController
and makes it easier to test the functionality now in the
simplify_stats_for_graphs method.

2 files changed, 135 insertions, 23 deletions

diff --git a/app/controllers/public_body_controller.rb b/app/controllers/public_body_controller.rb
index da80356f3..9c3e46ded 100644
--- a/app/controllers/public_body_controller.rb
+++ b/app/controllers/public_body_controller.rb
@@ -196,6 +196,50 @@ class PublicBodyController < ApplicationController
                   :disposition =>'attachment', :encoding => 'utf8')
     end
 
+
+    # This is a helper method to take data returned by the PublicBody
+    # model's statistics-generating methods, and converting them to
+    # simpler data structure that can be rendered by a Javascript
+    # graph library. (This could be a class method except that we need
+    # access to the URL helper public_body_path.)
+    def simplify_stats_for_graphs(data,
+                                  column,
+                                  percentages,
+                                  graph_properties)
+        # Copy the data, only taking known-to-be-safe keys:
+        result = Hash.new { |h, k| h[k] = [] }
+        result.update Hash[data.select do |key, value|
+            ['y_values',
+             'y_max',
+             'totals',
+             'cis_below',
+             'cis_above'].include? key
+        end]
+
+        # Extract data about the public bodies for the x-axis,
+        # tooltips, and so on:
+        data['public_bodies'].each_with_index do |pb, i|
+            result['x_values'] << i
+            result['x_ticks'] << [i, pb.name]
+            result['tooltips'] << "#{pb.name} (#{result['totals'][i]})"
+            result['public_bodies'] << {
+                'name' => pb.name,
+                'url' => public_body_path(pb)
+            }
+        end
+
+        # Set graph metadata properties, like the title, axis labels, etc.
+        graph_id = "#{column}-"
+        graph_id += graph_properties[:highest] ? 'highest' : 'lowest'
+        result.update({
+            'id' => graph_id,
+            'x_axis' => _('Public Bodies'),
+            'y_axis' => graph_properties[:y_axis],
+            'errorbars' => percentages,
+            'title' => graph_properties[:title]
+        })
+    end
+
     def statistics
         unless AlaveteliConfiguration::public_body_statistics_page
             raise ActiveRecord::RecordNotFound.new("Page not enabled")
@@ -252,29 +296,10 @@ class PublicBodyController < ApplicationController
                 end
 
                 if data
-                    # We just need the URL and name of each public body:
-                    data['public_bodies'].map! { |pb|
-                        {'name' => pb.name, 'url' => public_body_path(pb)}
-                    }
-
-                    data_to_draw = Hash.new { |h, k| h[k] = [] }
-                    data_to_draw.update({
-                        'id' => "#{column}-#{highest ? 'highest' : 'lowest'}",
-                        'x_axis' => _('Public Bodies'),
-                        'y_axis' => graph_properties[:y_axis],
-                        'errorbars' => percentages,
-                        'title' => graph_properties[:title]
-                    })
-
-                    data_to_draw.update(data)
-                    data['public_bodies'].each_with_index { |pb, i|
-                        data_to_draw['x_values'].push i
-                        data_to_draw['x_ticks'].push [i, pb['name']]
-                        tooltip = "#{pb['name']} (#{data_to_draw['totals'][i]})"
-                        data_to_draw['tooltips'].push tooltip
-                    }
-
-                    @graph_list.push data_to_draw
+                    @graph_list.push simplify_stats_for_graphs(data,
+                                                               column,
+                                                               percentages,
+                                                               graph_properties)
                 end
             end
         end
diff --git a/spec/controllers/public_body_controller_spec.rb b/spec/controllers/public_body_controller_spec.rb
index 34caa40ea..6800765f2 100644
--- a/spec/controllers/public_body_controller_spec.rb
+++ b/spec/controllers/public_body_controller_spec.rb
@@ -308,6 +308,93 @@ describe PublicBodyController, "when showing public body statistics" do
 
 end
 
+describe PublicBodyController, "when converting data for graphing" do
+
+    before(:each) do
+        @raw_count_data = PublicBody.get_request_totals(n=3,
+                                                        highest=true,
+                                                        minimum_requests=1)
+        @percentages_data = PublicBody.get_request_percentages(
+            column='info_requests_successful_count',
+            n=3,
+            highest=false,
+            minimum_requests=1)
+    end
+
+    it "should not include the real public body model instance" do
+        to_draw = controller.simplify_stats_for_graphs(@raw_count_data,
+                                                       column='blah_blah',
+                                                       percentages=false,
+                                                       {} )
+        to_draw['public_bodies'][0].class.should == Hash
+        to_draw['public_bodies'][0].has_key?('request_email').should be_false
+    end
+
+    it "should generate the expected id" do
+        to_draw = controller.simplify_stats_for_graphs(@raw_count_data,
+                                                       column='blah_blah',
+                                                       percentages=false,
+                                                       {:highest => true} )
+        to_draw['id'].should == "blah_blah-highest"
+        to_draw = controller.simplify_stats_for_graphs(@raw_count_data,
+                                                       column='blah_blah',
+                                                       percentages=false,
+                                                       {:highest => false} )
+        to_draw['id'].should == "blah_blah-lowest"
+    end
+
+    it "should have exactly the expected keys" do
+        to_draw = controller.simplify_stats_for_graphs(@raw_count_data,
+                                                       column='blah_blah',
+                                                       percentages=false,
+                                                       {} )
+        to_draw.keys.sort.should == ["errorbars", "id", "public_bodies",
+                                     "title", "tooltips", "totals",
+                                     "x_axis", "x_ticks", "x_values",
+                                     "y_axis", "y_max", "y_values"]
+
+        to_draw = controller.simplify_stats_for_graphs(@percentages_data,
+                                                       column='whatever',
+                                                       percentages=true,
+                                                       {})
+        to_draw.keys.sort.should == ["cis_above", "cis_below",
+                                     "errorbars", "id", "public_bodies",
+                                     "title", "tooltips", "totals",
+                                     "x_axis", "x_ticks", "x_values",
+                                     "y_axis", "y_max", "y_values"]
+    end
+
+    it "should have values of the expected class and length" do
+        [controller.simplify_stats_for_graphs(@raw_count_data,
+                                              column='blah_blah',
+                                              percentages=false,
+                                              {}),
+         controller.simplify_stats_for_graphs(@percentages_data,
+                                              column='whatever',
+                                              percentages=true,
+                                              {})].each do |to_draw|
+            per_pb_keys = ["cis_above", "cis_below", "public_bodies",
+                           "tooltips", "totals", "x_ticks", "x_values",
+                           "y_values"]
+            # These should be all be arrays with one element per public body:
+            per_pb_keys.each do |key|
+                if to_draw.has_key? key
+                    to_draw[key].class.should == Array
+                    to_draw[key].length.should eq(3), "for key #{key}"
+                end
+            end
+            # Just check that the rest aren't of class Array:
+            to_draw.keys.each do |key|
+                unless per_pb_keys.include? key
+                    to_draw[key].class.should_not eq(Array), "for key #{key}"
+                end
+            end
+        end
+    end
+
+end
+
+
 describe PublicBodyController, "when doing type ahead searches" do
 
     render_views