Fix the command-line CSV importer under Ruby 1.9

Under Ruby 1.8.7, you can parse a CSV file with the
following code (Example A):

  require 'csv'
  CSV.parse('foo.csv') do |row|
      puts "got row: #{row.inspect}"
  end

Rather confusingly, under Ruby 1.8.7, CSV.parse can also
take a string representation of the contents of the file
as its parameter, so this also works (Example B):

  require 'csv'
  CSV.parse("1,hello,red\n2,goodbye,green") do |row|
      puts "got row: #{row.inspect}"
  end

However under Ruby 1.9.3, CSV.parse only expects a string
representation of the contents of the CSV file, so only
Example B works; Example B fails silently (interpreting
the filename as a single cell CSV file, typically).

The import:import_csv rake task unfortunately relied on
both A and B working.  This commit fixes this by adding
PublicBody.import_csv_from_file, and refactoring
PublicBody.import_csv to use the newly added class method,
and adds a test to check for any regression in this
behaviour.

(This means that the usage of import_csv in the admin public
body controller's import_csv action could now be changed
to use PublicBody.import_csv_from_file directly from the
uploaded file, which would be more efficient and cope
with larger files without using lots of memory.)

Fixes #1229

1 files changed, 19 insertions, 4 deletions

diff --git a/app/models/public_body.rb b/app/models/public_body.rb
index 8e474c797..eb0905f9e 100644
--- a/app/models/public_body.rb
+++ b/app/models/public_body.rb
@@ -369,10 +369,24 @@ class PublicBody < ActiveRecord::Base
     class ImportCSVDryRun < StandardError
     end
 
-    # Import from CSV. Just tests things and returns messages if dry_run is true.
-    # Returns an array of [array of errors, array of notes]. If there are errors,
-    # always rolls back (as with dry_run).
+    # Import from a string in CSV format.
+    # Just tests things and returns messages if dry_run is true.
+    # Returns an array of [array of errors, array of notes]. If there
+    # are errors, always rolls back (as with dry_run).
     def self.import_csv(csv, tag, tag_behaviour, dry_run, editor, available_locales = [])
+        tmp_csv = nil
+        Tempfile.open('alaveteli') do |f|
+            f.write csv
+            tmp_csv = f
+        end
+        PublicBody.import_csv_from_file(tmp_csv.path, tag, tag_behaviour, dry_run, editor, available_locales)
+    end
+
+    # Import from a CSV file.
+    # Just tests things and returns messages if dry_run is true.
+    # Returns an array of [array of errors, array of notes]. If there
+    # are errors, always rolls back (as with dry_run).
+    def self.import_csv_from_file(csv_filename, tag, tag_behaviour, dry_run, editor, available_locales = [])
         errors = []
         notes = []
         available_locales = [I18n.default_locale] if available_locales.empty?
@@ -398,7 +412,8 @@ class PublicBody < ActiveRecord::Base
                 set_of_importing = Set.new()
                 field_names = { 'name'=>1, 'request_email'=>2 }     # Default values in case no field list is given
                 line = 0
-                CSV.parse(csv) do |row|
+
+                CSV.foreach(csv_filename) do |row|
                     line = line + 1
 
                     # Parse the first line as a field list if it starts with '#'