Extract public body CSV export to its own class

- SRP: Do one thing. PublicBodyCSV converts a
  collection of bodies in to a CSV formatted
  String
- Adds some parenthesis around parameters in
  PublicBodyController#list_all_csv
- Let the controller handle what records to pull
  out for the CSV export

Arguably this doesn't really need to be anything
to do with PublicBody, but it allows us to set
nice defaults.

5 files changed, 232 insertions, 50 deletions

diff --git a/app/controllers/public_body_controller.rb b/app/controllers/public_body_controller.rb
index 9f2edd0df..9e34396bc 100644
--- a/app/controllers/public_body_controller.rb
+++ b/app/controllers/public_body_controller.rb
@@ -190,6 +190,9 @@ class PublicBodyController < ApplicationController
         redirect_to list_public_bodies_url(:tag => @tag)
     end
 
+    # GET /body/all-authorities.csv
+    #
+    # Returns all public bodies (except for the internal admin authority) as CSV
     def list_all_csv
         # FIXME: this is just using the download directory for zip
         # archives, since we know that is allowed for X-Sendfile and
@@ -197,21 +200,29 @@ class PublicBodyController < ApplicationController
         # used for the zips.  However, really there should be a
         # generically named downloads directory that contains all
         # kinds of downloadable assets.
-        download_directory = File.join(InfoRequest.download_zip_dir(),
-                                       'download')
-        FileUtils.mkdir_p download_directory
+        download_directory = File.join(InfoRequest.download_zip_dir, 'download')
+        FileUtils.mkdir_p(download_directory)
         output_leafname = 'all-authorities.csv'
-        output_filename = File.join download_directory, output_leafname
+        output_filename = File.join(download_directory, output_leafname)
         # Create a temporary file in the same directory, so we can
         # rename it atomically to the intended filename:
-        tmp = Tempfile.new output_leafname, download_directory
+        tmp = Tempfile.new(output_leafname, download_directory)
         tmp.close
-        # Export all the public bodies to that temporary path and make
-        # it readable:
-        PublicBody.export_csv tmp.path
-        FileUtils.chmod 0644, tmp.path
-        # Rename into place and send the file:
-        File.rename tmp.path, output_filename
+
+        # Create the CSV
+        csv = PublicBodyCSV.new
+        PublicBody.visible.find_each(:include => [:translations, :tags]) do |public_body|
+           next if public_body.has_tag?('site_administration')
+           csv << public_body
+        end
+
+        # Export all the public bodies to that temporary path, make it readable,
+        # and rename it
+        File.open(tmp.path, 'w') { |file| file.write(csv.generate) }
+        FileUtils.chmod(0644, tmp.path)
+        File.rename(tmp.path, output_filename)
+
+        # Send the file
         send_file(output_filename,
                   :type => 'text/csv; charset=utf-8; header=present',
                   :filename => 'all-authorities.csv',
diff --git a/app/models/public_body.rb b/app/models/public_body.rb
index 03ec270ee..d0e5bbb9e 100644
--- a/app/models/public_body.rb
+++ b/app/models/public_body.rb
@@ -569,45 +569,6 @@ class PublicBody < ActiveRecord::Base
         return [errors, notes]
     end
 
-    # Returns all public bodies (except for the internal admin authority) as csv
-    def self.export_csv(output_filename)
-        CSV.open(output_filename, "w") do |csv|
-            csv << [
-                    'Name',
-                    'Short name',
-                    # deliberately not including 'Request email'
-                    'URL name',
-                    'Tags',
-                    'Home page',
-                    'Publication scheme',
-                    'Disclosure log',
-                    'Notes',
-                    'Created at',
-                    'Updated at',
-                    'Version',
-            ]
-            PublicBody.visible.find_each(:include => [:translations, :tags]) do |public_body|
-                # Skip bodies we use only for site admin
-                next if public_body.has_tag?('site_administration')
-                csv << [
-                    public_body.name,
-                    public_body.short_name,
-                    # DO NOT include request_email (we don't want to make it
-                    # easy to spam all authorities with requests)
-                    public_body.url_name,
-                    public_body.tag_string,
-                    public_body.calculated_home_page,
-                    public_body.publication_scheme,
-                    public_body.disclosure_log,
-                    public_body.notes,
-                    public_body.created_at,
-                    public_body.updated_at,
-                    public_body.version,
-                ]
-            end
-        end
-    end
-
     # Does this user have the power of FOI officer for this body?
     def is_foi_officer?(user)
         user_domain = user.email_domain
diff --git a/config/initializers/alaveteli.rb b/config/initializers/alaveteli.rb
index 6fb6b1420..d9879b5d1 100644
--- a/config/initializers/alaveteli.rb
+++ b/config/initializers/alaveteli.rb
@@ -53,6 +53,7 @@ require 'message_prominence'
 require 'theme'
 require 'xapian_queries'
 require 'date_quarter'
+require 'public_body_csv'
 
 AlaveteliLocalization.set_locales(AlaveteliConfiguration::available_locales,
                                   AlaveteliConfiguration::default_locale)
diff --git a/lib/public_body_csv.rb b/lib/public_body_csv.rb
new file mode 100644
index 000000000..afb5d9043
--- /dev/null
+++ b/lib/public_body_csv.rb
@@ -0,0 +1,95 @@
+require 'csv'
+
+# Public: Generate a CSV representation of PublicBody instances
+#
+# Examples
+#
+#   bodies = PublicBody.search('useless')
+#
+#   csv = PublicBodyCSV.new(:fields => [:name, :calculated_home_page],
+#                           :headers => ['Name', 'Home Page'])
+#
+#   bodies.each { |body| csv << body }
+#
+#   csv.generate
+#   # => Name,Home Page
+#        Department for Humpadinking,http://localhost
+#        Ministry of Silly Walks,http://www.localhost
+#        Department of Loneliness,http://localhost
+class PublicBodyCSV
+
+    def self.default_fields
+        [:name,
+         :short_name,
+         :url_name,
+         :tag_string,
+         :calculated_home_page,
+         :publication_scheme,
+         :disclosure_log,
+         :notes,
+         :created_at,
+         :updated_at,
+         :version]
+    end
+
+    # TODO: Generate headers from fields
+    def self.default_headers
+        ['Name',
+         'Short name',
+         'URL name',
+         'Tags',
+         'Home page',
+         'Publication scheme',
+         'Disclosure log',
+         'Notes',
+         'Created at',
+         'Updated at',
+         'Version']
+    end
+
+    attr_reader :fields, :headers, :rows
+
+    def initialize(args = {})
+        @fields = args.fetch(:fields, self.class.default_fields)
+        @headers = args.fetch(:headers, self.class.default_headers)
+        @rows = []
+    end
+
+    def <<(public_body)
+        # Allow join_rows to handle newlines because of differences between
+        # CSV.generate_line in 1.8 / 1.9+
+        if RUBY_VERSION.to_f >= 1.9
+            rows << CSV.generate_line(collect_public_body_attributes(public_body), :row_sep => '')
+        else
+            rows << CSV.generate_line(collect_public_body_attributes(public_body))
+        end
+    end
+
+    # TODO: Just use CSV.generate when Ruby 1.8.7 support is dropped
+    def generate
+        csv = generate_header_row
+        csv << join_rows
+        csv << "\n"
+    end
+
+    private
+
+    def join_rows
+        rows.join("\n")
+    end
+
+    def generate_header_row
+        # Add a newline because of differences between
+        # CSV.generate_line in 1.8 / 1.9+
+        row = CSV.generate_line(headers)
+        row += "\n" unless RUBY_VERSION.to_f >= 1.9
+        row
+    end
+
+    def collect_public_body_attributes(public_body)
+        fields.map do |field|
+            public_body.respond_to?(field) ? public_body.send(field) : ''
+        end
+    end
+
+end
diff --git a/spec/lib/public_body_csv_spec.rb b/spec/lib/public_body_csv_spec.rb
new file mode 100644
index 000000000..e3cc4be6e
--- /dev/null
+++ b/spec/lib/public_body_csv_spec.rb
@@ -0,0 +1,114 @@
+require File.expand_path(File.dirname(__FILE__) + '/../spec_helper')
+
+describe PublicBodyCSV do
+
+    describe '.default_fields' do
+        defaults = [:name,
+                    :short_name,
+                    :url_name,
+                    :tag_string,
+                    :calculated_home_page,
+                    :publication_scheme,
+                    :disclosure_log,
+                    :notes,
+                    :created_at,
+                    :updated_at,
+                    :version]
+        PublicBodyCSV.default_fields.should == defaults
+    end
+
+    describe '.default_headers' do
+        defaults = ['Name',
+                    'Short name',
+                    'URL name',
+                    'Tags',
+                    'Home page',
+                    'Publication scheme',
+                    'Disclosure log',
+                    'Notes',
+                    'Created at',
+                    'Updated at',
+                    'Version']
+        PublicBodyCSV.default_headers.should == defaults
+    end
+
+    describe :fields do
+
+        it 'has a default set of fields' do
+            csv = PublicBodyCSV.new
+            csv.fields.should == PublicBodyCSV.default_fields
+        end
+
+        # DO NOT include request_email (we don't want to make it
+        # easy to spam all authorities with requests)
+        it 'does not include the request_email attribute' do
+            csv = PublicBodyCSV.new
+            csv.fields.should_not include(:request_email)
+        end
+
+        it 'allows custom fields to be set on instantiation' do
+            custom_fields = [:name, :short_name]
+            csv = PublicBodyCSV.new(:fields => custom_fields)
+            csv.fields.should == custom_fields
+        end
+
+    end
+
+    describe :headers do
+
+        it 'has a default set of headers' do
+            csv = PublicBodyCSV.new
+            csv.headers.should == PublicBodyCSV.default_headers
+        end
+
+        it 'allows custom headers to be set on instantiation' do
+            custom_headers = ['Name', 'Short Name']
+            csv = PublicBodyCSV.new(:headers => custom_headers)
+            csv.headers.should == custom_headers
+        end
+
+    end
+
+    describe :rows do
+
+        it 'is empty on instantiation' do
+            csv = PublicBodyCSV.new
+            csv.rows.should be_empty
+        end
+
+    end
+
+    describe :<< do
+
+        it 'adds an elements attributes to the rows collection' do
+            csv = PublicBodyCSV.new
+            expected = ["Ministry of Silly Walks,MSW,msw,useless_agency,http://www.localhost,\"\",\"\",You know the one.,2007-10-25 10:51:01 UTC,2007-10-25 10:51:01 UTC,1"]
+            csv << PublicBody.find(5)
+            csv.rows.should == expected
+        end
+
+    end
+
+    describe :generate do
+
+        it 'generates the csv' do
+            expected = <<-CSV
+Name,Short name,URL name,Home page,Publication scheme,Disclosure log,Notes,Created at,Updated at,Version
+Department for Humpadinking,DfH,dfh,http://www.localhost,"","",An albatross told me!!!,2007-10-25 10:51:01 UTC,2007-10-25 10:51:01 UTC,2
+Department of Loneliness,DoL,lonely,http://www.localhost,"","",A very lonely public body that no one has corresponded with,2011-01-26 14:11:02 UTC,2011-01-26 14:11:02 UTC,1
+CSV
+
+            # Miss out the tags field because the specs keep changing the order
+            # that the tags are returned in
+            fields = [:name, :short_name, :url_name, :calculated_home_page, :publication_scheme, :disclosure_log, :notes, :created_at, :updated_at, :version]
+            headers = ['Name', 'Short name', 'URL name', 'Home page', 'Publication scheme', 'Disclosure log', 'Notes', 'Created at', 'Updated at', 'Version']
+
+            csv = PublicBodyCSV.new(:fields => fields, :headers => headers)
+            csv << PublicBody.where(:name => 'Department for Humpadinking').first
+            csv << PublicBody.where(:name => 'Department of Loneliness').first
+            csv.generate.should == expected
+        end
+
+    end
+
+end