diff options
-rw-r--r-- | app/controllers/request_controller.rb | 2 | ||||
-rw-r--r-- | app/models/info_request.rb | 4 | ||||
-rw-r--r-- | app/models/raw_email.rb | 39 | ||||
-rw-r--r-- | app/views/request/_followup.rhtml | 4 | ||||
-rw-r--r-- | config/general.yml-example | 4 | ||||
-rw-r--r-- | config/routes.rb | 1 | ||||
-rw-r--r-- | db/migrate/099_move_raw_email_to_filesystem.rb | 23 | ||||
-rw-r--r-- | db/migrate/100_remove_redundant_raw_email_columns.rb | 12 | ||||
-rw-r--r-- | spec/controllers/request_controller_spec.rb | 52 | ||||
-rw-r--r-- | spec/controllers/request_game_controller_spec.rb | 3 | ||||
-rw-r--r-- | spec/controllers/track_controller_spec.rb | 5 | ||||
-rw-r--r-- | spec/controllers/user_controller_spec.rb | 3 | ||||
-rw-r--r-- | spec/fixtures/files/useless_raw_email.email | 15 | ||||
-rw-r--r-- | spec/fixtures/raw_emails.yml | 17 | ||||
-rw-r--r-- | spec/models/incoming_message_spec.rb | 6 | ||||
-rw-r--r-- | spec/models/outgoing_mailer_spec.rb | 12 | ||||
-rw-r--r-- | spec/models/raw_email_spec.rb | 4 | ||||
-rw-r--r-- | spec/models/request_mailer_spec.rb | 3 | ||||
-rw-r--r-- | spec/models/xapian_spec.rb | 18 | ||||
-rw-r--r-- | spec/spec_helper.rb | 11 |
20 files changed, 203 insertions, 35 deletions
diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb index 472f18f6e..7b9421464 100644 --- a/app/controllers/request_controller.rb +++ b/app/controllers/request_controller.rb @@ -313,7 +313,7 @@ class RequestController < ApplicationController replied by then.</p> <p>If you write about this request (for example in a forum or a blog) please link to this page, and add an annotation below telling people about your writing.</p>",:law_used_full=>@info_request.law_used_full) - redirect_to request_url(@info_request) + redirect_to show_new_request_path(:url_title => @info_request.url_title) end # Submitted to the describing state of messages form diff --git a/app/models/info_request.rb b/app/models/info_request.rb index de3b5bdc5..209954b16 100644 --- a/app/models/info_request.rb +++ b/app/models/info_request.rb @@ -428,11 +428,11 @@ public ActiveRecord::Base.transaction do raw_email = RawEmail.new - raw_email.data = raw_email_data incoming_message.raw_email = raw_email incoming_message.info_request = self - raw_email.save! incoming_message.save! + raw_email.data = raw_email_data + raw_email.save! self.awaiting_description = true self.log_event("response", { :incoming_message_id => incoming_message.id }) diff --git a/app/models/raw_email.rb b/app/models/raw_email.rb index 2406cee7c..1d85cc7d9 100644 --- a/app/models/raw_email.rb +++ b/app/models/raw_email.rb @@ -20,17 +20,54 @@ class RawEmail < ActiveRecord::Base has_one :incoming_message + before_destroy :destroy_file_representation! # We keep the old data_text field (which is of type text) for backwards # compatibility. We use the new data_binary field because only it works # properly in recent versions of PostgreSQL (get seg faults escaping # some binary strings). + def directory + request_id = self.incoming_message.info_request.id.to_s + if ENV["RAILS_ENV"] = "test" + return 'files/raw_email_test' + else + return File.join(MySociety::Config.get('RAW_EMAILS_LOCATION', + 'files/raw_emails'), + request_id[0..2], request_id) + end + end + + def filepath + File.join(self.directory, self.incoming_message.id.to_s) + end + def data=(d) - write_attribute(:data_binary, d) + if !File.exists?(self.directory) + FileUtils.mkdir_p self.directory + end + File.open(self.filepath, "wb") { |file| + file.write d + } end def data + if !File.exists?(self.filepath) + dbdata + else + File.open(self.filepath, "rb" ).read + end + end + + def destroy_file_representation! + File.delete(self.filepath) + end + + def dbdata=(d) + write_attribute(:data_binary, d) + end + + def dbdata d = read_attribute(:data_binary) if !d.nil? return d diff --git a/app/views/request/_followup.rhtml b/app/views/request/_followup.rhtml index 8c279d234..78de7decd 100644 --- a/app/views/request/_followup.rhtml +++ b/app/views/request/_followup.rhtml @@ -25,9 +25,7 @@ <% end %> <p> - <%= _('Please <strong>only</strong> write messages directly relating to your - request {{request_link}}. If you would like to ask for information - that was not in your original request, then <a href="%s">file a new request</a>.',:request_link=>request_link(@info_request)) % [new_request_to_body_url(:url_name => @info_request.public_body.url_name)] %> + <%= _('Please <strong>only</strong> write messages directly relating to your request {{request_link}}. If you would like to ask for information that was not in your original request, then <a href="{{new_request_link}}">file a new request</a>.', :request_link=>request_link(@info_request), :new_request_link => new_request_to_body_url(:url_name => @info_request.public_body.url_name)) %> </p> <% status = @info_request.calculate_status %> diff --git a/config/general.yml-example b/config/general.yml-example index 157c2e23b..9db95c4d0 100644 --- a/config/general.yml-example +++ b/config/general.yml-example @@ -56,6 +56,10 @@ ADMIN_PASSWORD: 'passwordx' CONTACT_EMAIL: 'postmaster@localhost' CONTACT_NAME: 'Alaveteli Webmaster' +# Where the raw incoming email data gets stored; make sure you back +# this up! +RAW_EMAILS_LOCATION: 'files/raw_emails' + # The base URL for admin pages. # If not specified, it will default to the path to the admin controller, # which is usually what you want. It is useful in situations where admin diff --git a/config/routes.rb b/config/routes.rb index 631be07ba..d04547b11 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -39,6 +39,7 @@ ActionController::Routing::Routes.draw do |map| request.new_request_to_body '/new/:url_name', :action => 'new' request.show_request '/request/:url_title.:format', :action => 'show' + request.show_new_request '/request/:url_title/new', :action => 'show' request.details_request '/details/request/:url_title', :action => 'details' request.similar_request '/similar/request/:url_title', :action => 'similar' diff --git a/db/migrate/099_move_raw_email_to_filesystem.rb b/db/migrate/099_move_raw_email_to_filesystem.rb new file mode 100644 index 000000000..ea77580e1 --- /dev/null +++ b/db/migrate/099_move_raw_email_to_filesystem.rb @@ -0,0 +1,23 @@ +class MoveRawEmailToFilesystem < ActiveRecord::Migration + def self.up + batch_size = 10 + 0.step(RawEmail.count, batch_size) do |i| + RawEmail.find(:all, :limit => batch_size, :offset => i, :order => :id).each do |raw_email| + if !File.exists?(raw_email.filepath) + STDERR.puts "converting raw_email " + raw_email.id.to_s + raw_email.data = raw_email.dbdata + #raw_email.dbdata = nil + #raw_email.save! + end + end + end + end + + def self.down + raise "safer not to have reverse migration scripts, and we never use them" + end +end + + + + diff --git a/db/migrate/100_remove_redundant_raw_email_columns.rb b/db/migrate/100_remove_redundant_raw_email_columns.rb new file mode 100644 index 000000000..edf6006d7 --- /dev/null +++ b/db/migrate/100_remove_redundant_raw_email_columns.rb @@ -0,0 +1,12 @@ +class RemoveRedundantRawEmailColumns < ActiveRecord::Migration + def self.up + remove_column :raw_emails, :data_text + remove_column :raw_emails, :data_binary + end + def self.down + end +end + + + + diff --git a/spec/controllers/request_controller_spec.rb b/spec/controllers/request_controller_spec.rb index 9d91bf8c2..69c1433f2 100644 --- a/spec/controllers/request_controller_spec.rb +++ b/spec/controllers/request_controller_spec.rb @@ -8,7 +8,7 @@ require 'json' describe RequestController, "when listing recent requests" do - before(:all) do + before(:each) do rebuild_xapian_index end @@ -35,11 +35,14 @@ describe RequestController, "when listing recent requests" do end end - describe RequestController, "when showing one request" do fixtures :info_requests, :info_request_events, :public_bodies, :public_body_translations, :public_body_translations, :users, :incoming_messages, :raw_emails, :outgoing_messages, :comments # all needed as integrating views - + + before(:each) do + load_raw_emails_data(raw_emails) + end + it "should be successful" do get :show, :url_title => 'why_do_you_have_such_a_fancy_dog' response.should be_success @@ -198,6 +201,10 @@ end describe RequestController, "when changing prominence of a request" do fixtures :info_requests, :info_request_events, :public_bodies, :public_body_translations, :users, :incoming_messages, :raw_emails, :outgoing_messages # all needed as integrating views + before(:each) do + load_raw_emails_data(raw_emails) + end + it "should not show hidden requests" do ir = info_requests(:fancy_dog_request) ir.prominence = 'hidden' @@ -363,6 +370,9 @@ describe RequestController, "when creating a new request" do mail.body.should =~ /This is a silly letter. It is too short to be interesting./ response.should redirect_to(:action => 'show', :url_title => ir.url_title) + # This test uses an explicit path because it's relied in + # Google Analytics goals: + response.redirected_to.should =~ /request\/why_is_your_quango_called_gerald\/new$/ end it "should give an error if the same request is submitted twice" do @@ -454,7 +464,11 @@ end describe RequestController, "when viewing an individual response for reply/followup" do integrate_views fixtures :info_requests, :info_request_events, :public_bodies, :public_body_translations, :users, :incoming_messages, :raw_emails, :outgoing_messages, :comments # all needed as integrating views - + + before(:each) do + load_raw_emails_data(raw_emails) + end + it "should ask for login if you are logged in as wrong person" do session[:user_id] = users(:silly_name_user).id get :show_response, :id => info_requests(:fancy_dog_request).id, :incoming_message_id => incoming_messages(:useless_incoming_message) @@ -486,6 +500,7 @@ describe RequestController, "when classifying an information request" do @dog_request = info_requests(:fancy_dog_request) @dog_request.stub!(:is_old_unclassified?).and_return(false) InfoRequest.stub!(:find).and_return(@dog_request) + load_raw_emails_data(raw_emails) end def post_status(status) @@ -895,7 +910,11 @@ end describe RequestController, "sending overdue request alerts" do integrate_views fixtures :info_requests, :info_request_events, :public_bodies, :public_body_translations, :users, :incoming_messages, :raw_emails, :outgoing_messages # all needed as integrating views - + + before(:each) do + load_raw_emails_data(raw_emails) + end + it "should send an overdue alert mail to creators of overdue requests" do chicken_request = info_requests(:naughty_chicken_request) chicken_request.outgoing_messages[0].last_sent_at = Time.now() - 30.days @@ -979,7 +998,11 @@ end describe RequestController, "sending unclassified new response reminder alerts" do integrate_views fixtures :info_requests, :info_request_events, :public_bodies, :public_body_translations, :users, :incoming_messages, :raw_emails, :outgoing_messages, :comments # all needed as integrating views - + + before(:each) do + load_raw_emails_data(raw_emails) + end + it "should send an alert" do RequestMailer.alert_new_response_reminders @@ -1006,7 +1029,10 @@ end describe RequestController, "clarification required alerts" do integrate_views fixtures :info_requests, :info_request_events, :public_bodies, :public_body_translations, :users, :incoming_messages, :raw_emails, :outgoing_messages # all needed as integrating views - + before(:each) do + load_raw_emails_data(raw_emails) + end + it "should send an alert" do ir = info_requests(:fancy_dog_request) ir.set_described_state('waiting_clarification') @@ -1057,6 +1083,9 @@ end describe RequestController, "comment alerts" do integrate_views fixtures :info_requests, :info_request_events, :public_bodies, :public_body_translations, :users, :incoming_messages, :raw_emails, :outgoing_messages, :comments # all needed as integrating views + before(:each) do + load_raw_emails_data(raw_emails) + end it "should send an alert (once and once only)" do # delete ficture comment and make new one, so is in last month (as @@ -1128,7 +1157,10 @@ end describe RequestController, "when viewing comments" do integrate_views - fixtures :users + fixtures :users, :raw_emails, :incoming_messages, :info_requests + before(:each) do + load_raw_emails_data(raw_emails) + end it "should link to the user who submitted it" do session[:user_id] = users(:bob_smith_user).id @@ -1237,6 +1269,10 @@ describe RequestController, "when showing JSON version for API" do fixtures :info_requests, :info_request_events, :public_bodies, :public_body_translations, :users, :incoming_messages, :raw_emails, :outgoing_messages, :comments + before(:each) do + load_raw_emails_data(raw_emails) + end + it "should return data in JSON form" do get :show, :url_title => 'why_do_you_have_such_a_fancy_dog', :format => 'json' diff --git a/spec/controllers/request_game_controller_spec.rb b/spec/controllers/request_game_controller_spec.rb index ce507fad1..4883bfdd6 100644 --- a/spec/controllers/request_game_controller_spec.rb +++ b/spec/controllers/request_game_controller_spec.rb @@ -3,6 +3,9 @@ require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') describe RequestGameController, "when playing the game" do fixtures :info_requests, :info_request_events, :public_bodies, :public_body_translations, :users, :incoming_messages, :raw_emails, :outgoing_messages # all needed as integrating views + before(:each) do + load_raw_emails_data(raw_emails) + end it "should show the game homepage" do get :play diff --git a/spec/controllers/track_controller_spec.rb b/spec/controllers/track_controller_spec.rb index 0a8919268..f7002a9d4 100644 --- a/spec/controllers/track_controller_spec.rb +++ b/spec/controllers/track_controller_spec.rb @@ -37,6 +37,9 @@ describe TrackController, "when sending alerts for a track" do integrate_views fixtures :info_requests, :outgoing_messages, :incoming_messages, :raw_emails, :info_request_events, :users, :track_things, :track_things_sent_emails, :public_bodies, :public_body_translations include LinkToHelper # for main_url + before(:each) do + load_raw_emails_data(raw_emails) + end before do rebuild_xapian_index @@ -100,6 +103,7 @@ describe TrackController, "when viewing RSS feed for a track" do before do rebuild_xapian_index + load_raw_emails_data(raw_emails) end it "should get the RSS feed" do @@ -125,6 +129,7 @@ describe TrackController, "when viewing JSON version of a track feed" do before do rebuild_xapian_index + load_raw_emails_data(raw_emails) end it "should get the feed" do diff --git a/spec/controllers/user_controller_spec.rb b/spec/controllers/user_controller_spec.rb index c6817fbc6..0b5e96711 100644 --- a/spec/controllers/user_controller_spec.rb +++ b/spec/controllers/user_controller_spec.rb @@ -8,6 +8,9 @@ require 'json' describe UserController, "when showing a user" do integrate_views fixtures :users, :outgoing_messages, :incoming_messages, :raw_emails, :info_requests, :info_request_events, :comments, :public_bodies, :public_body_translations + before(:each) do + load_raw_emails_data(raw_emails) + end it "should be successful" do get :show, :url_name => "bob_smith" diff --git a/spec/fixtures/files/useless_raw_email.email b/spec/fixtures/files/useless_raw_email.email new file mode 100644 index 000000000..2e4585af7 --- /dev/null +++ b/spec/fixtures/files/useless_raw_email.email @@ -0,0 +1,15 @@ +From: "FOI Person" <foiperson@localhost> +To: "Bob Smith" <bob@localhost> +Date: Tue, 13 Nov 2007 11:39:55 +0000 +Bcc: +Subject: Geraldine FOI Code AZXB421 +Reply-To: +In-Reply-To: <471f1eae5d1cb_7347..fdbe67386163@cat.tmail> + +No way! I'm not going to tell you that in a month of Thursdays. + +The Geraldine Quango + +On Wed, Oct 24, 2007 at 11:30:06AM +0100, Bob Smith wrote: +> Why do you have such a fancy dog? + diff --git a/spec/fixtures/raw_emails.yml b/spec/fixtures/raw_emails.yml index d24b61854..8ef9248a3 100644 --- a/spec/fixtures/raw_emails.yml +++ b/spec/fixtures/raw_emails.yml @@ -1,19 +1,2 @@ useless_raw_email: id: 1 - data_binary: | - From: "FOI Person" <foiperson@localhost> - To: "Bob Smith" <bob@localhost> - Date: Tue, 13 Nov 2007 11:39:55 +0000 - NoCc: foi+request-1-4b571715@cat - Bcc: - Subject: Geraldine FOI Code AZXB421 - Reply-To: - In-Reply-To: <471f1eae5d1cb_7347..fdbe67386163@cat.tmail> - - No way! I'm not going to tell you that in a month of Thursdays. - - The Geraldine Quango - - On Wed, Oct 24, 2007 at 11:30:06AM +0100, Bob Smith wrote: - > Why do you have such a fancy dog? - diff --git a/spec/models/incoming_message_spec.rb b/spec/models/incoming_message_spec.rb index abbe94a47..42ea748fd 100644 --- a/spec/models/incoming_message_spec.rb +++ b/spec/models/incoming_message_spec.rb @@ -1,10 +1,11 @@ require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') describe IncomingMessage, " when dealing with incoming mail" do - fixtures :incoming_messages, :raw_emails + fixtures :incoming_messages, :raw_emails, :info_requests before do @im = incoming_messages(:useless_incoming_message) + load_raw_emails_data(raw_emails) end it "should return the mail Date header date for sent at" do @@ -129,6 +130,8 @@ describe IncomingMessage, " when censoring data" do @censor_rule_2.last_edit_editor = "unknown" @censor_rule_2.last_edit_comment = "none" @im.info_request.censor_rules << @censor_rule_2 + + load_raw_emails_data(raw_emails) end it "should do nothing to a JPEG" do @@ -212,6 +215,7 @@ describe IncomingMessage, " when censoring whole users" do @censor_rule_1.last_edit_editor = "unknown" @censor_rule_1.last_edit_comment = "none" @im.info_request.user.censor_rules << @censor_rule_1 + load_raw_emails_data(raw_emails) end it "should apply censor rules to HTML files" do diff --git a/spec/models/outgoing_mailer_spec.rb b/spec/models/outgoing_mailer_spec.rb index 3073c5ffd..d7587cb41 100644 --- a/spec/models/outgoing_mailer_spec.rb +++ b/spec/models/outgoing_mailer_spec.rb @@ -5,6 +5,9 @@ describe OutgoingMailer, " when working out follow up addresses" do # calls TMail. XXX untangle it and make these tests spread out and using # mocks. Put parts of the tests in spec/lib/tmail_extensions.rb fixtures :info_requests, :incoming_messages, :raw_emails, :public_bodies, :public_body_translations + before(:each) do + load_raw_emails_data(raw_emails) + end it "should parse them right" do ir = info_requests(:fancy_dog_request) @@ -67,8 +70,12 @@ describe OutgoingMailer, " when working out follow up addresses" do end describe OutgoingMailer, "when working out follow up subjects" do - fixtures :info_requests, :incoming_messages, :outgoing_messages - + fixtures :info_requests, :incoming_messages, :outgoing_messages, :raw_emails + + before(:each) do + load_raw_emails_data(raw_emails) + end + it "should prefix the title with 'Freedom of Information request -' for initial requests" do ir = info_requests(:fancy_dog_request) im = ir.incoming_messages[0] @@ -105,6 +112,7 @@ describe OutgoingMailer, "when working out follow up subjects" do it "should not add Re: prefix if there already is a lower case re: prefix" do ir = info_requests(:fancy_dog_request) im = ir.incoming_messages[0] + puts im.raw_email.data om = outgoing_messages(:useless_outgoing_message) om.incoming_message_followup = im diff --git a/spec/models/raw_email_spec.rb b/spec/models/raw_email_spec.rb index 6f3a8acd6..ff2830a62 100644 --- a/spec/models/raw_email_spec.rb +++ b/spec/models/raw_email_spec.rb @@ -3,6 +3,10 @@ require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') describe User, "manipulating a raw email" do before do @raw_email = RawEmail.new + incoming_message = mock_model(IncomingMessage) + info_request = mock_model(InfoRequest) + incoming_message.stub!(:info_request).and_return(info_request) + @raw_email.stub!(:incoming_message).and_return(incoming_message) end it 'putting data in comes back out' do diff --git a/spec/models/request_mailer_spec.rb b/spec/models/request_mailer_spec.rb index a8311e8f8..a5f59b9bf 100644 --- a/spec/models/request_mailer_spec.rb +++ b/spec/models/request_mailer_spec.rb @@ -2,6 +2,9 @@ require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') describe RequestMailer, " when receiving incoming mail" do fixtures :info_requests, :incoming_messages, :raw_emails, :users, :public_bodies, :public_body_translations + before(:each) do + load_raw_emails_data(raw_emails) + end it "should append it to the appropriate request" do ir = info_requests(:fancy_dog_request) diff --git a/spec/models/xapian_spec.rb b/spec/models/xapian_spec.rb index 79ffc4839..37e68b145 100644 --- a/spec/models/xapian_spec.rb +++ b/spec/models/xapian_spec.rb @@ -35,6 +35,9 @@ end describe PublicBody, " when indexing public bodies with Xapian" do fixtures :public_bodies, :public_body_translations, :incoming_messages, :outgoing_messages, :raw_emails, :comments + before(:each) do + load_raw_emails_data(raw_emails) + end it "should search index the main name field" do rebuild_xapian_index @@ -126,6 +129,9 @@ end describe User, " when indexing requests by user they are from" do fixtures :users, :info_request_events, :info_requests, :incoming_messages, :outgoing_messages, :raw_emails, :comments + before(:each) do + load_raw_emails_data(raw_emails) + end it "should find requests from the user" do rebuild_xapian_index @@ -210,6 +216,9 @@ end describe User, " when indexing comments by user they are by" do fixtures :users, :info_request_events, :info_requests, :comments, :incoming_messages, :outgoing_messages, :raw_emails, :comments + before(:each) do + load_raw_emails_data(raw_emails) + end it "should find requests from the user" do rebuild_xapian_index @@ -244,6 +253,9 @@ end describe InfoRequest, " when indexing requests by their title" do fixtures :info_request_events, :info_requests, :incoming_messages, :raw_emails, :comments + before(:each) do + load_raw_emails_data(raw_emails) + end it "should find events for the request" do rebuild_xapian_index @@ -272,6 +284,9 @@ end describe InfoRequest, " when indexing requests by tag" do fixtures :info_request_events, :info_requests, :incoming_messages, :raw_emails, :comments + before(:each) do + load_raw_emails_data(raw_emails) + end it "should find request by tag, even when changes" do rebuild_xapian_index @@ -291,6 +306,9 @@ end describe PublicBody, " when indexing authorities by tag" do fixtures :public_bodies, :public_body_translations, :incoming_messages, :outgoing_messages, :raw_emails, :comments + before(:each) do + load_raw_emails_data(raw_emails) + end it "should find request by tag, even when changes" do rebuild_xapian_index diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 5e6d9ec4a..42c5ff6bf 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -13,6 +13,7 @@ config['ADMIN_PASSWORD'] = 'baz' # tests assume 20 days config['REPLY_LATE_AFTER_DAYS'] = 20 + # Uncomment the next line to use webrat's matchers #require 'webrat/integrations/rspec-rails' @@ -26,6 +27,7 @@ Spec::Runner.configure do |config| # in your config/boot.rb config.fixture_path = RAILS_ROOT + '/spec/fixtures/' + # == Fixtures # # You can declare fixtures for each example_group like this: @@ -144,3 +146,12 @@ if $tempfilecount.nil? puts "WARNING: HTML validation script " + $html_validation_script + " not found" end end + +def load_raw_emails_data(raw_emails) + raw_email = raw_emails(:useless_raw_email) + begin + raw_email.destroy_file_representation! + rescue Errno::ENOENT + end + raw_email.data = load_file_fixture("useless_raw_email.email") +end |