diff options
| -rw-r--r-- | app/controllers/admin_public_body_controller.rb | 2 | ||||
| -rw-r--r-- | app/models/public_body.rb | 84 | ||||
| -rw-r--r-- | app/views/admin_public_body/import_csv.rhtml | 7 | ||||
| -rw-r--r-- | spec/fixtures/files/fake-authority-type-with-field-names.csv | 6 | ||||
| -rw-r--r-- | spec/models/public_body_spec.rb | 49 |
5 files changed, 83 insertions, 65 deletions
diff --git a/app/controllers/admin_public_body_controller.rb b/app/controllers/admin_public_body_controller.rb index 021122734..13232024c 100644 --- a/app/controllers/admin_public_body_controller.rb +++ b/app/controllers/admin_public_body_controller.rb @@ -166,7 +166,7 @@ class AdminPublicBodyController < AdminController notes.push("Dry run was successful, real run would do as above.") else # And if OK, with real run - en = PublicBody.import_csv(csv_contents, params[:tag], false, admin_http_auth_user(), available_locales) + en = PublicBody.import_csv(csv_contents, params[:tag], false, admin_http_auth_user(), I18n.available_locales) errors = en[0] notes = en[1] if errors.size != 0 diff --git a/app/models/public_body.rb b/app/models/public_body.rb index b75da4331..94d34952b 100644 --- a/app/models/public_body.rb +++ b/app/models/public_body.rb @@ -311,9 +311,10 @@ class PublicBody < ActiveRecord::Base # 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). - def self.import_csv(csv, tag, dry_run, editor, additional_locales = []) + def self.import_csv(csv, tag, dry_run, editor, available_locales = []) errors = [] notes = [] + available_locales = [I18n.default_locale] if available_locales.empty? begin ActiveRecord::Base.transaction do @@ -330,7 +331,7 @@ class PublicBody < ActiveRecord::Base end set_of_importing = Set.new() - field_names = { 'name'=>1, 'email'=>2 } # Default values in case no field list is given + field_names = { 'name'=>1, 'request_email'=>2 } # Default values in case no field list is given line = 0 CSV::Reader.parse(csv) do |row| line = line + 1 @@ -343,55 +344,60 @@ class PublicBody < ActiveRecord::Base end name = row[field_names['name']] - email = row[field_names['email']] + email = row[field_names['request_email']] next if name.nil? - if email.nil? - email = '' # unknown/bad contact is empty string - end name.strip! email.strip! - if email != "" && !MySociety::Validate.is_valid_email(email) - errors.push "error: line " + line.to_s + ": invalid email " + email + " for authority '" + name + "'" + if !email.nil? && !email.empty? && !MySociety::Validate.is_valid_email(email) + errors.push "error: line #{line.to_s}: invalid email '#{email}' for authority '#{name}'" next end + + field_list = ['name', 'short_name', 'request_email', 'notes', 'publication_scheme', 'home_page'] + + if public_body = bodies_by_name[name] + available_locales.each do |locale| + PublicBody.with_locale(locale) do + changed = {} + field_list.each do |field_name| + localized_field_name = (locale === I18n.default_locale) ? field_name : "#{field_name}.#{locale}" + localized_value = field_names[localized_field_name] && row[field_names[localized_field_name]] + if !localized_value.nil? and public_body.send(field_name) != localized_value + changed[field_name] = "#{public_body.send(field_name)}: #{localized_value}" + public_body.send("#{field_name}=", localized_value) + end + end - if bodies_by_name[name] - # Already have the public body, just update email - public_body = bodies_by_name[name] - if public_body.request_email != email - notes.push "line " + line.to_s + ": updating email for '" + name + "' from " + public_body.request_email + " to " + email - public_body.request_email = email - public_body.last_edit_editor = editor - public_body.last_edit_comment = 'Updated from spreadsheet' - public_body.save! - end - - additional_locales.each do |locale| - localized_name = field_names["name.#{locale}"] && row[field_names["name.#{locale}"]] - PublicBody.with_locale(locale) do - if !localized_name.nil? and public_body.name != localized_name - notes.push "line " + line.to_s + ": updating name for '#{name}' from '#{public_body.name}' to '#{localized_name}' (locale: #{locale})." - public_body.name = localized_name + unless changed.empty? + notes.push "line #{line.to_s}: updating authority '#{name}' (locale: #{locale}):\n\t#{changed.to_json}" + public_body.last_edit_editor = editor + public_body.last_edit_comment = 'Updated from spreadsheet' public_body.save! end end end - else - # New public body - notes.push "line " + line.to_s + ": new authority '" + name + "' with email " + email - public_body = PublicBody.new(:name => name, :request_email => email, :short_name => "", :home_page => "", :publication_scheme => "", :notes => "", :last_edit_editor => editor, :last_edit_comment => 'Created from spreadsheet') - public_body.tag_string = tag - public_body.save! - - additional_locales.each do |locale| - localized_name = field_names["name.#{locale}"] && row[field_names["name.#{locale}"]] - if !localized_name.nil? - PublicBody.with_locale(locale) do - notes.push "line " + line.to_s + ": (aka '#{localized_name}' in locale #{locale})" - public_body.name = localized_name - public_body.publication_scheme = "" + else # New public body + public_body = PublicBody.new(:name=>name, :short_name=>"", :request_email=>"") + available_locales.each do |locale| + PublicBody.with_locale(locale) do + changed = {} + field_list.each do |field_name| + localized_field_name = (locale === I18n.default_locale) ? field_name : "#{field_name}.#{locale}" + localized_value = field_names[localized_field_name] && row[field_names[localized_field_name]] + if !localized_value.nil? and public_body.send(field_name) != localized_value + changed[field_name] = localized_value + public_body.send("#{field_name}=", localized_value) + end + end + + unless changed.empty? + notes.push "line #{line.to_s}: creating new authority '#{name}' (locale: #{locale}):\n\t#{changed.to_json}" + public_body.publication_scheme = public_body.publication_scheme || "" + public_body.tag_string = tag + public_body.last_edit_editor = editor + public_body.last_edit_comment = 'Created from spreadsheet' public_body.save! end end diff --git a/app/views/admin_public_body/import_csv.rhtml b/app/views/admin_public_body/import_csv.rhtml index 50a4b951a..3bcc4bf41 100644 --- a/app/views/admin_public_body/import_csv.rhtml +++ b/app/views/admin_public_body/import_csv.rhtml @@ -23,16 +23,19 @@ <p><strong>CSV file format:</strong> A first row with the list of fields, starting with '#', is optional but highly recommended. The fields 'name' - and 'email' are required; additionaly, translated values are supported by + and 'request_email' are required; additionaly, translated values are supported by adding the locale name to the field name, e.g. 'name.es', 'name.de'... Example: </p> <blockquote> - #id,name,email,name.es<br/> + #id,name,request_email,name.es<br/> 1,An Authority,a@example.com,Un organismo<br/> 2,Another One,another@example.com,Otro organismo<br/> </blockquote> + <p>Supported files: name (i18n), short_name (i18n), request_email (i18n), notes (i18n), + publication_scheme (i18n), home_page.</p> + <p><strong>Note:</strong> Choose <strong>dry run</strong> to test, without actually altering the database. Choose <strong>upload</strong> to actually make the changes. In either case, you will be shown any errors, or details diff --git a/spec/fixtures/files/fake-authority-type-with-field-names.csv b/spec/fixtures/files/fake-authority-type-with-field-names.csv index 93ce00a25..9e82308fb 100644 --- a/spec/fixtures/files/fake-authority-type-with-field-names.csv +++ b/spec/fixtures/files/fake-authority-type-with-field-names.csv @@ -1,4 +1,4 @@ -#id,email,name,name.es -,north_west_foi@localhost,North West Fake Authority,Autoridad del Nordeste -,scottish_foi@localhost,Scottish Fake Authority,Autoridad Escocesa +#id,request_email,name,name.es,home_page +,north_west_foi@localhost,North West Fake Authority,Autoridad del Nordeste,http://northwest.org +,scottish_foi@localhost,Scottish Fake Authority,Autoridad Escocesa,http://scottish.org ,ni_foi@localhost,Fake Authority of Northern Ireland,Autoridad Irlandesa diff --git a/spec/models/public_body_spec.rb b/spec/models/public_body_spec.rb index 3d00d37fb..3902e3662 100644 --- a/spec/models/public_body_spec.rb +++ b/spec/models/public_body_spec.rb @@ -234,9 +234,11 @@ describe PublicBody, " when loading CSV files" do errors, notes = PublicBody.import_csv(csv_contents, 'fake', true, 'someadmin') # true means dry run errors.should == [] notes.size.should == 3 - notes.should == ["line 1: new authority 'North West Fake Authority' with email north_west_foi@localhost", - "line 2: new authority 'Scottish Fake Authority' with email scottish_foi@localhost", - "line 3: new authority 'Fake Authority of Northern Ireland' with email ni_foi@localhost"] + notes.should == [ + "line 1: creating new authority 'North West Fake Authority' (locale: en):\n\t\{\"request_email\":\"north_west_foi@localhost\"\}", + "line 2: creating new authority 'Scottish Fake Authority' (locale: en):\n\t\{\"request_email\":\"scottish_foi@localhost\"\}", + "line 3: creating new authority 'Fake Authority of Northern Ireland' (locale: en):\n\t\{\"request_email\":\"ni_foi@localhost\"\}" + ] PublicBody.count.should == original_count end @@ -248,9 +250,11 @@ describe PublicBody, " when loading CSV files" do errors, notes = PublicBody.import_csv(csv_contents, 'fake', false, 'someadmin') # false means real run errors.should == [] notes.size.should == 3 - notes.should == ["line 1: new authority 'North West Fake Authority' with email north_west_foi@localhost", - "line 2: new authority 'Scottish Fake Authority' with email scottish_foi@localhost", - "line 3: new authority 'Fake Authority of Northern Ireland' with email ni_foi@localhost"] + notes.should == [ + "line 1: creating new authority 'North West Fake Authority' (locale: en):\n\t\{\"request_email\":\"north_west_foi@localhost\"\}", + "line 2: creating new authority 'Scottish Fake Authority' (locale: en):\n\t\{\"request_email\":\"scottish_foi@localhost\"\}", + "line 3: creating new authority 'Fake Authority of Northern Ireland' (locale: en):\n\t\{\"request_email\":\"ni_foi@localhost\"\}" + ] PublicBody.count.should == original_count + 3 end @@ -262,9 +266,11 @@ describe PublicBody, " when loading CSV files" do errors, notes = PublicBody.import_csv(csv_contents, 'fake', true, 'someadmin') # true means dry run errors.should == [] notes.size.should == 3 - notes.should == ["line 2: new authority 'North West Fake Authority' with email north_west_foi@localhost", - "line 3: new authority 'Scottish Fake Authority' with email scottish_foi@localhost", - "line 4: new authority 'Fake Authority of Northern Ireland' with email ni_foi@localhost"] + notes.should == [ + "line 2: creating new authority 'North West Fake Authority' (locale: en):\n\t\{\"request_email\":\"north_west_foi@localhost\",\"home_page\":\"http://northwest.org\"\}", + "line 3: creating new authority 'Scottish Fake Authority' (locale: en):\n\t\{\"request_email\":\"scottish_foi@localhost\",\"home_page\":\"http://scottish.org\"\}", + "line 4: creating new authority 'Fake Authority of Northern Ireland' (locale: en):\n\t\{\"request_email\":\"ni_foi@localhost\"\}" + ] PublicBody.count.should == original_count end @@ -273,16 +279,17 @@ describe PublicBody, " when loading CSV files" do original_count = PublicBody.count csv_contents = load_file_fixture("fake-authority-type-with-field-names.csv") - errors, notes = PublicBody.import_csv(csv_contents, 'fake', false, 'someadmin', ['es']) + errors, notes = PublicBody.import_csv(csv_contents, 'fake', false, 'someadmin', [:en, :es]) errors.should == [] notes.size.should == 6 notes.should == [ - "line 2: new authority 'North West Fake Authority' with email north_west_foi@localhost", - "line 2: (aka 'Autoridad del Nordeste' in locale es)", - "line 3: new authority 'Scottish Fake Authority' with email scottish_foi@localhost", - "line 3: (aka 'Autoridad Escocesa' in locale es)", - "line 4: new authority 'Fake Authority of Northern Ireland' with email ni_foi@localhost", - "line 4: (aka 'Autoridad Irlandesa' in locale es)"] + "line 2: creating new authority 'North West Fake Authority' (locale: en):\n\t{\"request_email\":\"north_west_foi@localhost\",\"home_page\":\"http://northwest.org\"}", + "line 2: creating new authority 'North West Fake Authority' (locale: es):\n\t{\"name\":\"Autoridad del Nordeste\"}", + "line 3: creating new authority 'Scottish Fake Authority' (locale: en):\n\t{\"request_email\":\"scottish_foi@localhost\",\"home_page\":\"http://scottish.org\"}", + "line 3: creating new authority 'Scottish Fake Authority' (locale: es):\n\t{\"name\":\"Autoridad Escocesa\"}", + "line 4: creating new authority 'Fake Authority of Northern Ireland' (locale: en):\n\t{\"request_email\":\"ni_foi@localhost\"}", + "line 4: creating new authority 'Fake Authority of Northern Ireland' (locale: es):\n\t{\"name\":\"Autoridad Irlandesa\"}" + ] PublicBody.count.should == original_count + 3 @@ -297,12 +304,14 @@ describe PublicBody, " when loading CSV files" do original_count = PublicBody.count csv_contents = load_file_fixture("fake-authority-type-with-field-names.csv") - errors, notes = PublicBody.import_csv(csv_contents, 'fake', true, 'someadmin', ['xx']) # true means dry run + errors, notes = PublicBody.import_csv(csv_contents, 'fake', true, 'someadmin', [:en, :xx]) # true means dry run errors.should == [] notes.size.should == 3 - notes.should == ["line 2: new authority 'North West Fake Authority' with email north_west_foi@localhost", - "line 3: new authority 'Scottish Fake Authority' with email scottish_foi@localhost", - "line 4: new authority 'Fake Authority of Northern Ireland' with email ni_foi@localhost"] + notes.should == [ + "line 2: creating new authority 'North West Fake Authority' (locale: en):\n\t{\"request_email\":\"north_west_foi@localhost\",\"home_page\":\"http://northwest.org\"}", + "line 3: creating new authority 'Scottish Fake Authority' (locale: en):\n\t{\"request_email\":\"scottish_foi@localhost\",\"home_page\":\"http://scottish.org\"}", + "line 4: creating new authority 'Fake Authority of Northern Ireland' (locale: en):\n\t{\"request_email\":\"ni_foi@localhost\"}" + ] PublicBody.count.should == original_count end |