diff options
-rw-r--r-- | app/controllers/admin_controller.rb | 2 | ||||
-rw-r--r-- | app/controllers/admin_public_body_controller.rb | 2 | ||||
-rw-r--r-- | app/controllers/admin_request_controller.rb | 8 | ||||
-rw-r--r-- | app/controllers/api_controller.rb | 2 | ||||
-rw-r--r-- | app/controllers/application_controller.rb | 11 | ||||
-rw-r--r-- | app/controllers/request_controller.rb | 2 | ||||
-rw-r--r-- | app/models/info_request.rb | 44 | ||||
-rw-r--r-- | app/views/admin_general/timeline.html.erb | 2 | ||||
-rw-r--r-- | config/initializers/alaveteli.rb | 2 | ||||
-rw-r--r-- | config/packages | 6 | ||||
-rw-r--r-- | doc/CHANGES.md | 16 | ||||
-rw-r--r-- | doc/INSTALL.md | 2 | ||||
-rw-r--r-- | lib/tasks/temp.rake | 55 | ||||
-rwxr-xr-x | script/load-sample-data | 10 | ||||
-rwxr-xr-x | script/rails-post-deploy | 4 | ||||
-rw-r--r-- | spec/controllers/api_controller_spec.rb | 3 | ||||
-rw-r--r-- | spec/fixtures/files/fake-authority-type.csv | 1 | ||||
-rw-r--r-- | spec/models/info_request_spec.rb | 53 | ||||
-rw-r--r-- | spec/models/public_body_spec.rb | 32 |
19 files changed, 196 insertions, 61 deletions
diff --git a/app/controllers/admin_controller.rb b/app/controllers/admin_controller.rb index 0bccd3358..f5191504e 100644 --- a/app/controllers/admin_controller.rb +++ b/app/controllers/admin_controller.rb @@ -17,7 +17,7 @@ class AdminController < ApplicationController end # Always give full stack trace for admin interface - def local_request? + def show_rails_exceptions? true end diff --git a/app/controllers/admin_public_body_controller.rb b/app/controllers/admin_public_body_controller.rb index 078af12f4..ec2a08dbc 100644 --- a/app/controllers/admin_public_body_controller.rb +++ b/app/controllers/admin_public_body_controller.rb @@ -146,12 +146,12 @@ class AdminPublicBodyController < AdminController if params[:csv_file] csv_contents = params[:csv_file].read @original_csv_file = params[:csv_file].original_filename + csv_contents = normalize_string_to_utf8(csv_contents) # or from previous dry-run temporary file elsif params[:temporary_csv_file] && params[:original_csv_file] csv_contents = retrieve_csv_data(params[:temporary_csv_file]) @original_csv_file = params[:original_csv_file] end - if !csv_contents.nil? # Try with dry run first errors, notes = PublicBody.import_csv(csv_contents, diff --git a/app/controllers/admin_request_controller.rb b/app/controllers/admin_request_controller.rb index 66989ea93..40ccfb98c 100644 --- a/app/controllers/admin_request_controller.rb +++ b/app/controllers/admin_request_controller.rb @@ -62,9 +62,6 @@ class AdminRequestController < AdminController @info_request.title = params[:info_request][:title] @info_request.prominence = params[:info_request][:prominence] - if @info_request.described_state != params[:info_request][:described_state] - @info_request.set_described_state(params[:info_request][:described_state]) - end @info_request.awaiting_description = params[:info_request][:awaiting_description] == "true" ? true : false @info_request.allow_new_responses_from = params[:info_request][:allow_new_responses_from] @info_request.handle_rejected_responses = params[:info_request][:handle_rejected_responses] @@ -77,13 +74,16 @@ class AdminRequestController < AdminController { :editor => admin_current_user(), :old_title => old_title, :title => @info_request.title, :old_prominence => old_prominence, :prominence => @info_request.prominence, - :old_described_state => old_described_state, :described_state => @info_request.described_state, + :old_described_state => old_described_state, :described_state => params[:info_request][:described_state], :old_awaiting_description => old_awaiting_description, :awaiting_description => @info_request.awaiting_description, :old_allow_new_responses_from => old_allow_new_responses_from, :allow_new_responses_from => @info_request.allow_new_responses_from, :old_handle_rejected_responses => old_handle_rejected_responses, :handle_rejected_responses => @info_request.handle_rejected_responses, :old_tag_string => old_tag_string, :tag_string => @info_request.tag_string, :old_comments_allowed => old_comments_allowed, :comments_allowed => @info_request.comments_allowed }) + if @info_request.described_state != params[:info_request][:described_state] + @info_request.set_described_state(params[:info_request][:described_state]) + end # expire cached files expire_for_request(@info_request) flash[:notice] = 'Request successfully updated.' diff --git a/app/controllers/api_controller.rb b/app/controllers/api_controller.rb index 49b226e4b..e7bea67ef 100644 --- a/app/controllers/api_controller.rb +++ b/app/controllers/api_controller.rb @@ -63,6 +63,8 @@ class ApiController < ApplicationController :smtp_message_id => nil ) + request.set_described_state('waiting_response') + # Return the URL and ID number. render :json => { 'url' => make_url("request", request.url_title), diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 88b107861..2ce44011f 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -119,12 +119,9 @@ class ApplicationController < ActionController::Base end def render_exception(exception) - - # In development, or the admin interface, or for a local request, let Rails handle the exception - # with its stack trace templates. Local requests in testing are a special case so that we can - # test this method - there we use consider_all_requests_local to control behaviour. - if Rails.application.config.consider_all_requests_local || local_request? || - (request.local? && !Rails.env.test?) + # In development or the admin interface let Rails handle the exception + # with its stack trace templates + if Rails.application.config.consider_all_requests_local || show_rails_exceptions? raise exception end @@ -150,7 +147,7 @@ class ApplicationController < ActionController::Base end end - def local_request? + def show_rails_exceptions? false end diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb index 0c1d9880c..45d8b7de6 100644 --- a/app/controllers/request_controller.rb +++ b/app/controllers/request_controller.rb @@ -593,7 +593,7 @@ class RequestController < ApplicationController @outgoing_message.set_signature_name(@user.name) if !@user.nil? if (not @incoming_message.nil?) and @info_request != @incoming_message.info_request - raise sprintf("Incoming message %d does not belong to request %d", @incoming_message.info_request_id, @info_request.id) + raise ActiveRecord::RecordNotFound.new("Incoming message #{@incoming_message.id} does not belong to request #{@info_request.id}") end # Test for hidden requests diff --git a/app/models/info_request.rb b/app/models/info_request.rb index 8f15a4ea4..9bce2ca88 100644 --- a/app/models/info_request.rb +++ b/app/models/info_request.rb @@ -563,12 +563,15 @@ public end # change status, including for last event for later historical purposes + # described_state should always indicate the current state of the request, as described + # by the request owner (or, in some other cases an admin or other user) def set_described_state(new_state, set_by = nil, message = "") old_described_state = described_state ActiveRecord::Base.transaction do self.awaiting_description = false last_event = self.info_request_events.last last_event.described_state = new_state + self.described_state = new_state last_event.save! self.save! @@ -588,11 +591,14 @@ public end end - # Work out what the situation of the request is. In addition to values of - # self.described_state, can take these two values: + # Work out what state to display for the request on the site. In addition to values of + # self.described_state, can take these values: # waiting_classification # waiting_response_overdue # waiting_response_very_overdue + # (this method adds an assessment of overdueness with respect to the current time to 'waiting_response' + # states, and will return 'waiting_classification' instead of the described_state if the + # awaiting_description flag is set on the request). def calculate_status(cached_value_ok=false) if cached_value_ok && @cached_calculated_status return @cached_calculated_status @@ -611,10 +617,22 @@ public return 'waiting_response' end + + # 'described_state' can be populated on any info_request_event but is only + # ever used in the process populating calculated_state on the + # info_request_event (if it represents a response, outgoing message, edit + # or status update), or previous response or outgoing message events for + # the same request. + # Fill in any missing event states for first response before a description # was made. i.e. We take the last described state in between two responses # (inclusive of earlier), and set it as calculated value for the earlier - # response. + # response. Also set the calculated state for any initial outgoing message, + # follow up, edit or status_update to the described state of that event. + + # Note that the calculated state of the latest info_request_event will + # be used in latest_status based searches and should match the described_state + # of the info_request. def calculate_event_states curr_state = nil for event in self.info_request_events.reverse @@ -636,7 +654,7 @@ public event.save! end curr_state = nil - elsif !curr_state.nil? && (event.event_type == 'followup_sent' || event.event_type == 'sent' || event.event_type == "status_update") + elsif !curr_state.nil? && (event.event_type == 'followup_sent' || event.event_type == 'sent') && !event.described_state.nil? && (event.described_state == 'waiting_response' || event.described_state == 'internal_review') # Followups can set the status to waiting response / internal # review. Initial requests ('sent') set the status to waiting response. @@ -648,10 +666,22 @@ public event.save! end - # And we don't want to propogate it to the response itself, + # And we don't want to propagate it to the response itself, # as that might already be set to waiting_clarification / a # success status, which we want to know about. curr_state = nil + elsif !curr_state.nil? && (['edit', 'status_update'].include? event.event_type) + # A status update or edit event should get the same calculated state as described state + # so that the described state is always indexed (and will be the latest_status + # for the request immediately after it has been described, regardless of what + # other request events precede it). This means that request should be correctly included + # in status searches for that status. These events allow the described state to propagate in + # case there is a preceding response that the described state should be applied to. + if event.calculated_state != event.described_state + event.calculated_state = event.described_state + event.last_described_at = Time.now() + event.save! + end end end end @@ -1107,10 +1137,10 @@ public begin if self.described_state.nil? self.described_state = 'waiting_response' - end + end rescue ActiveModel::MissingAttributeError # this should only happen on Model.exists?() call. It can be safely ignored. - # See http://www.tatvartha.com/2011/03/activerecordmissingattributeerror-missing-attribute-a-bug-or-a-features/ + # See http://www.tatvartha.com/2011/03/activerecordmissingattributeerror-missing-attribute-a-bug-or-a-features/ end # FOI or EIR? if !self.public_body.nil? && self.public_body.eir_only? diff --git a/app/views/admin_general/timeline.html.erb b/app/views/admin_general/timeline.html.erb index 8fd8875b6..439ae1e68 100644 --- a/app/views/admin_general/timeline.html.erb +++ b/app/views/admin_general/timeline.html.erb @@ -88,7 +88,7 @@ <% elsif event.event_type == 'comment' %> had an annotation posted by <%=h event.comment.user.name %>. <% elsif event.event_type == 'status_update' %> - had its status updated by <%=h User.find(event.params[:user_id]).name %> from '<%= h event.params[:old_described_state] %>' to '<%= h event.params[:described_state] %>'. + had its status updated by <%= event.params[:user_id] ? User.find(event.params[:user_id]).name : event.params[:script] %> from '<%= h event.params[:old_described_state] %>' to '<%= h event.params[:described_state] %>'. <% else %> had '<%=event.event_type%>' done to it, parameters <%=h event.params_yaml%>. <% end %> diff --git a/config/initializers/alaveteli.rb b/config/initializers/alaveteli.rb index deee3f6da..2c6ee2fbc 100644 --- a/config/initializers/alaveteli.rb +++ b/config/initializers/alaveteli.rb @@ -10,7 +10,7 @@ load "debug_helpers.rb" load "util.rb" # Application version -ALAVETELI_VERSION = '0.12' +ALAVETELI_VERSION = '0.13' # Add new inflection rules using the following format # (all these examples are active by default): diff --git a/config/packages b/config/packages index f4d0a674c..e89ede177 100644 --- a/config/packages +++ b/config/packages @@ -4,8 +4,8 @@ ruby1.8 ruby libopenssl-ruby1.8 # needed for Ubuntu 10.04 TLS; included in libruby1.8 in Squeeze -rdoc -irb +rdoc | rdoc1.8 +irb | irb1.8 wv poppler-utils pdftk (>> 1.41+dfsg-1) | pdftk (<< 1.41+dfsg-1) # that version has a non-functionining uncompress option @@ -28,6 +28,8 @@ wkhtmltopdf-static libmagic-dev libmagickwand-dev libpq-dev +libxml2-dev +libxslt-dev uuid-dev ruby1.8-dev rubygems diff --git a/doc/CHANGES.md b/doc/CHANGES.md index 7a22f44e2..4720e5283 100644 --- a/doc/CHANGES.md +++ b/doc/CHANGES.md @@ -1,3 +1,19 @@ +# Version 0.13 +## Highlighted features + +* Fix for bug that resulted in some incorrect results when using search by request status [issue #460](https://github.com/mysociety/alaveteli/issues/460). You can view and fix requests with inconsistent state history using `rake temp:fix_bad_request_states` +* All status updates (whether by the request owner or another user) are now logged in the event history, for better audit) (Matthew Landauer) +* Fix for bug that dropped accented characters from URLs [issue #282](https://github.com/mysociety/alaveteli/issues/282) (zejn) +* A fix for a bug that produced binary mask errors when handling multibyte characters in responses [issue #991](https://github.com/mysociety/alaveteli/issues/991) +* Some locale fixes for locales with a dash in them [issue #998](https://github.com/mysociety/alaveteli/issues/998) and [issue #999](https://github.com/mysociety/alaveteli/issues/999). +* Some improvements in the labelling of defunct authorities (Matthew Somerville) +* The addition of a check on the status of the commonlib submodule to the rails-post-deploy script. + +## Upgrade notes +* Check out this version and run `rails-post-deploy` as usual. +* This release includes an update to the commonlib submodule - you should now be warned about this on running `rails-post-deploy`. You can update to the nw version with `git submodule update`. +* After deploying, run `rake temp:fix_bad_request_states` to find and list requests that have an inconsistent history - run `rake temp:fix_bad_request_states DRYRUN=0` to fix them. + # Version 0.12 ## Highlighted features * Remove support for theme stylesheet inclusion via template (deprecated in version 0.5) diff --git a/doc/INSTALL.md b/doc/INSTALL.md index 1ef7ad592..ea8871888 100644 --- a/doc/INSTALL.md +++ b/doc/INSTALL.md @@ -1,4 +1,4 @@ -These instructions assume Debian Squeeze or Ubuntu 10.04 LTS. +These instructions assume Debian Squeeze (64-bit) or Ubuntu 10.04 LTS. [Install instructions for OS X](https://github.com/mysociety/alaveteli/wiki/OS-X-Quickstart) are under development. Debian Squeeze is the best supported deployment platform. diff --git a/lib/tasks/temp.rake b/lib/tasks/temp.rake index fcabb23de..f746338f0 100644 --- a/lib/tasks/temp.rake +++ b/lib/tasks/temp.rake @@ -1,5 +1,36 @@ namespace :temp do + desc "Fix the history of requests where the described state doesn't match the latest status value + used by search, by adding an edit event that will correct the latest status" + task :fix_bad_request_states => :environment do + dryrun = ENV['DRYRUN'] != '0' + if dryrun + puts "This is a dryrun" + end + + InfoRequest.find_each() do |info_request| + next if info_request.url_title == 'holding_pen' + last_info_request_event = info_request.info_request_events[-1] + if last_info_request_event.latest_status != info_request.described_state + puts "#{info_request.id} #{info_request.url_title} #{last_info_request_event.latest_status} #{info_request.described_state}" + params = { :script => 'rake temp:fix_bad_request_states', + :user_id => nil, + :old_described_state => info_request.described_state, + :described_state => info_request.described_state + } + if ! dryrun + info_request.info_request_events.create!(:last_described_at => last_info_request_event.described_at + 1.second, + :event_type => 'status_update', + :described_state => info_request.described_state, + :calculated_state => info_request.described_state, + :params => params) + info_request.info_request_events.each{ |event| event.mark_needs_xapian_index } + end + end + + end + end + def disable_duplicate_account(user, count, dryrun) dupe_email = "duplicateemail#{count}@example.com" puts "Updating #{user.email} to #{dupe_email} for user #{user.id}" @@ -13,15 +44,21 @@ namespace :temp do total_messages = 0 messages_to_reparse = 0 IncomingMessage.find_each :include => :foi_attachments do |im| - reparse = im.foi_attachments.any? { |fa| ! File.exists? fa.filepath } - total_messages += 1 - messages_to_reparse += 1 if reparse - if total_messages % 1000 == 0 - puts "Considered #{total_messages} received emails." - end - unless dry_run - im.parse_raw_email! true if reparse - sleep 2 + begin + reparse = im.foi_attachments.any? { |fa| ! File.exists? fa.filepath } + total_messages += 1 + messages_to_reparse += 1 if reparse + if total_messages % 1000 == 0 + puts "Considered #{total_messages} received emails." + end + unless dry_run + im.parse_raw_email! true if reparse + sleep 2 + end + rescue StandardError => e + puts "There was a #{e.class} exception reparsing IncomingMessage with ID #{im.id}" + puts e.backtrace + puts e.message end end message = dry_run ? "Would reparse" : "Reparsed" diff --git a/script/load-sample-data b/script/load-sample-data index e91516886..c9e5997f5 100755 --- a/script/load-sample-data +++ b/script/load-sample-data @@ -5,22 +5,20 @@ # have a filesystem representation of their contents export LOC=`dirname "$0"` - bundle exec rails runner /dev/stdin <<END require 'rspec/rails' -require "#{ENV['LOC']}/../spec/support/load_file_fixtures.rb" -require "#{ENV['LOC']}/../spec/support/email_helpers.rb" +require Rails.root.join("spec", "support", "load_file_fixtures") +require Rails.root.join("spec", "support", "email_helpers") RSpec.configure do |config| - config.fixture_path = "#{::Rails.root}/spec/fixtures" + config.fixture_path = Rails.root.join("spec","fixtures") end # HACK: Normally to load fixtures you'd run `rake db:fixtures:load` but since we # have .csv files in the fixtures folder Rails tries to load those too. Therefore # we've pinched some code to load the fixtures: # https://github.com/rails/rails/blob/v3.1.11/activerecord/lib/active_record/railties/databases.rake#L311 -fixtures_dir = "#{ENV['LOC']}/../spec/fixtures" - +fixtures_dir = Rails.root.join("spec","fixtures").to_s Dir["#{fixtures_dir}/**/*.yml"].each do |fixture_file| ActiveRecord::Fixtures.create_fixtures(fixtures_dir, fixture_file[(fixtures_dir.size + 1)..-5]) end diff --git a/script/rails-post-deploy b/script/rails-post-deploy index d20bf0472..6eca2f68f 100755 --- a/script/rails-post-deploy +++ b/script/rails-post-deploy @@ -91,11 +91,9 @@ then fi bundle install $bundle_install_options -bundle exec rake themes:install - bundle exec rake submodules:check # upgrade database bundle exec rake db:migrate #--trace - +bundle exec rake themes:install diff --git a/spec/controllers/api_controller_spec.rb b/spec/controllers/api_controller_spec.rb index 66b8e33f0..8e9d17fbe 100644 --- a/spec/controllers/api_controller_spec.rb +++ b/spec/controllers/api_controller_spec.rb @@ -83,6 +83,9 @@ describe ApiController, "when using the API" do new_request.last_event_forming_initial_request.outgoing_message.body.should == request_data["body"].strip new_request.public_body_id.should == public_bodies(:geraldine_public_body).id + new_request.info_request_events.size.should == 1 + new_request.info_request_events[0].event_type.should == 'sent' + new_request.info_request_events[0].calculated_state.should == 'waiting_response' end def _create_request diff --git a/spec/fixtures/files/fake-authority-type.csv b/spec/fixtures/files/fake-authority-type.csv index 4aa618ad1..cb25050c6 100644 --- a/spec/fixtures/files/fake-authority-type.csv +++ b/spec/fixtures/files/fake-authority-type.csv @@ -1,3 +1,4 @@ ,North West Fake Authority,north_west_foi@localhost ,Scottish Fake Authority,scottish_foi@localhost ,Fake Authority of Northern Ireland,ni_foi@localhost +,Gobierno de Aragón,spain_foi@localhost diff --git a/spec/models/info_request_spec.rb b/spec/models/info_request_spec.rb index c9ee57c74..3451e018f 100644 --- a/spec/models/info_request_spec.rb +++ b/spec/models/info_request_spec.rb @@ -670,7 +670,7 @@ describe InfoRequest do events[0].calculated_state.should == "waiting_response" events[1].event_type.should == "response" events[1].described_state.should be_nil - events[1].calculated_state.should be_nil + events[1].calculated_state.should == 'waiting_response' events[2].event_type.should == "status_update" events[2].described_state.should == "waiting_response" events[2].calculated_state.should == "waiting_response" @@ -698,7 +698,7 @@ describe InfoRequest do events[0].calculated_state.should == "waiting_response" events[1].event_type.should == "response" events[1].described_state.should be_nil - events[1].calculated_state.should be_nil + events[1].calculated_state.should == 'waiting_response' events[2].event_type.should == "status_update" events[2].described_state.should == "waiting_response" events[2].calculated_state.should == "waiting_response" @@ -731,7 +731,7 @@ describe InfoRequest do events[0].calculated_state.should == "waiting_response" events[1].event_type.should == "response" events[1].described_state.should be_nil - events[1].calculated_state.should be_nil + events[1].calculated_state.should == 'waiting_response' events[2].event_type.should == "status_update" events[2].described_state.should == "waiting_response" events[2].calculated_state.should == "waiting_response" @@ -807,6 +807,53 @@ describe InfoRequest do events[1].described_state.should == "successful" events[1].calculated_state.should == "successful" end + + it "should have sensible event states" do + # An initial request is sent + request.log_event('sent', {}) + request.set_described_state('waiting_response') + + # A response is received + request.awaiting_description = true + request.log_event("response", {}) + + # The user marks the request as successful + request.log_event("status_update", {}) + request.set_described_state("successful") + + events = request.info_request_events + events.count.should == 3 + events[0].event_type.should == "sent" + events[0].described_state.should == "waiting_response" + events[0].calculated_state.should == "waiting_response" + events[1].event_type.should == "response" + events[1].described_state.should be_nil + events[1].calculated_state.should == "successful" + events[2].event_type.should == "status_update" + events[2].described_state.should == "successful" + events[2].calculated_state.should == "successful" + end + end + + context "another series of events on a request", :focus => true do + it "should have sensible event states" do + # An initial request is sent + request.log_event('sent', {}) + request.set_described_state('waiting_response') + # An admin sets the status of the request to 'gone postal' using + # the admin interface + request.log_event("edit", {}) + request.set_described_state("gone_postal") + + events = request.info_request_events + events.count.should == 2 + events[0].event_type.should == "sent" + events[0].described_state.should == "waiting_response" + events[0].calculated_state.should == "waiting_response" + events[1].event_type.should == "edit" + events[1].described_state.should == "gone_postal" + events[1].calculated_state.should == "gone_postal" + end end end end diff --git a/spec/models/public_body_spec.rb b/spec/models/public_body_spec.rb index 34a91a2c9..90affaaaa 100644 --- a/spec/models/public_body_spec.rb +++ b/spec/models/public_body_spec.rb @@ -1,3 +1,4 @@ +# encoding: UTF-8 require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') describe PublicBody, " using tags" do @@ -266,16 +267,17 @@ describe PublicBody, " when loading CSV files" do it "should do a dry run successfully" do original_count = PublicBody.count - csv_contents = load_file_fixture("fake-authority-type.csv") + csv_contents = normalize_string_to_utf8(load_file_fixture("fake-authority-type.csv")) errors, notes = PublicBody.import_csv(csv_contents, '', 'replace', true, 'someadmin') # true means dry run errors.should == [] - notes.size.should == 4 - notes[0..2].should == [ + notes.size.should == 5 + notes[0..3].should == [ "line 1: creating new authority 'North West Fake Authority' (locale: en):\n\t\{\"name\":\"North West Fake Authority\",\"request_email\":\"north_west_foi@localhost\"\}", "line 2: creating new authority 'Scottish Fake Authority' (locale: en):\n\t\{\"name\":\"Scottish Fake Authority\",\"request_email\":\"scottish_foi@localhost\"\}", "line 3: creating new authority 'Fake Authority of Northern Ireland' (locale: en):\n\t\{\"name\":\"Fake Authority of Northern Ireland\",\"request_email\":\"ni_foi@localhost\"\}", + "line 4: creating new authority 'Gobierno de Aragón' (locale: en):\n\t\{\"name\":\"Gobierno de Arag\\u00f3n\",\"request_email\":\"spain_foi@localhost\"}", ] - notes[3].should =~ /Notes: Some bodies are in database, but not in CSV file:\n( [A-Za-z ]+\n)*You may want to delete them manually.\n/ + notes[4].should =~ /Notes: Some bodies are in database, but not in CSV file:\n( [A-Za-z ]+\n)*You may want to delete them manually.\n/ PublicBody.count.should == original_count end @@ -283,34 +285,36 @@ describe PublicBody, " when loading CSV files" do it "should do full run successfully" do original_count = PublicBody.count - csv_contents = load_file_fixture("fake-authority-type.csv") + csv_contents = normalize_string_to_utf8(load_file_fixture("fake-authority-type.csv")) errors, notes = PublicBody.import_csv(csv_contents, '', 'replace', false, 'someadmin') # false means real run errors.should == [] - notes.size.should == 4 - notes[0..2].should == [ + notes.size.should == 5 + notes[0..3].should == [ "line 1: creating new authority 'North West Fake Authority' (locale: en):\n\t\{\"name\":\"North West Fake Authority\",\"request_email\":\"north_west_foi@localhost\"\}", "line 2: creating new authority 'Scottish Fake Authority' (locale: en):\n\t\{\"name\":\"Scottish Fake Authority\",\"request_email\":\"scottish_foi@localhost\"\}", "line 3: creating new authority 'Fake Authority of Northern Ireland' (locale: en):\n\t\{\"name\":\"Fake Authority of Northern Ireland\",\"request_email\":\"ni_foi@localhost\"\}", + "line 4: creating new authority 'Gobierno de Aragón' (locale: en):\n\t\{\"name\":\"Gobierno de Arag\\u00f3n\",\"request_email\":\"spain_foi@localhost\"}", ] - notes[3].should =~ /Notes: Some bodies are in database, but not in CSV file:\n( [A-Za-z ]+\n)*You may want to delete them manually.\n/ + notes[4].should =~ /Notes: Some bodies are in database, but not in CSV file:\n( [A-Za-z ]+\n)*You may want to delete them manually.\n/ - PublicBody.count.should == original_count + 3 + PublicBody.count.should == original_count + 4 end it "should do imports without a tag successfully" do original_count = PublicBody.count - csv_contents = load_file_fixture("fake-authority-type.csv") + csv_contents = normalize_string_to_utf8(load_file_fixture("fake-authority-type.csv")) errors, notes = PublicBody.import_csv(csv_contents, '', 'replace', false, 'someadmin') # false means real run errors.should == [] - notes.size.should == 4 - notes[0..2].should == [ + notes.size.should == 5 + notes[0..3].should == [ "line 1: creating new authority 'North West Fake Authority' (locale: en):\n\t\{\"name\":\"North West Fake Authority\",\"request_email\":\"north_west_foi@localhost\"\}", "line 2: creating new authority 'Scottish Fake Authority' (locale: en):\n\t\{\"name\":\"Scottish Fake Authority\",\"request_email\":\"scottish_foi@localhost\"\}", "line 3: creating new authority 'Fake Authority of Northern Ireland' (locale: en):\n\t\{\"name\":\"Fake Authority of Northern Ireland\",\"request_email\":\"ni_foi@localhost\"\}", + "line 4: creating new authority 'Gobierno de Aragón' (locale: en):\n\t\{\"name\":\"Gobierno de Arag\\u00f3n\",\"request_email\":\"spain_foi@localhost\"}", ] - notes[3].should =~ /Notes: Some bodies are in database, but not in CSV file:\n( [A-Za-z ]+\n)*You may want to delete them manually.\n/ - PublicBody.count.should == original_count + 3 + notes[4].should =~ /Notes: Some bodies are in database, but not in CSV file:\n( [A-Za-z ]+\n)*You may want to delete them manually.\n/ + PublicBody.count.should == original_count + 4 end it "should handle a field list and fields out of order" do |