diff options
-rwxr-xr-x | INSTALL.txt | 11 | ||||
-rw-r--r-- | app/controllers/admin_request_controller.rb | 4 | ||||
-rw-r--r-- | app/controllers/request_controller.rb | 2 | ||||
-rw-r--r-- | app/models/incoming_message.rb | 8 | ||||
-rw-r--r-- | app/models/info_request.rb | 14 | ||||
-rw-r--r-- | app/models/outgoing_mailer.rb | 2 | ||||
-rw-r--r-- | app/views/help/about.rhtml | 39 | ||||
-rw-r--r-- | app/views/public_body/show.rhtml | 2 | ||||
-rw-r--r-- | config/environment.rb | 4 | ||||
-rw-r--r-- | config/packages | 6 | ||||
-rw-r--r-- | spec/controllers/request_controller_spec.rb | 86 | ||||
-rw-r--r-- | spec/fixtures/raw_emails.yml | 2 | ||||
-rw-r--r-- | spec/models/incoming_message_spec.rb | 3 | ||||
-rw-r--r-- | spec/models/outgoing_mailer_spec.rb | 47 | ||||
-rw-r--r-- | spec/spec_helper.rb | 2 | ||||
-rw-r--r-- | todo.txt | 18 |
16 files changed, 214 insertions, 36 deletions
diff --git a/INSTALL.txt b/INSTALL.txt index d22b1e607..6a4aadbb7 100755 --- a/INSTALL.txt +++ b/INSTALL.txt @@ -18,9 +18,13 @@ Commands are intended to be run via the terminal or over ssh. Firstly, in a terminal, navigate to the whatdotheyknow folder where this install guide lives. -Install the packages that are listed in config/packages using apt-get eg: +Install the packages that are listed in config/packages using apt-get e.g.: -> sudo apt-get install `cat config/packages` +> sudo apt-get install `cut -d " " -f 1 config/packages` + +Some of the files also have a version number listed in config/packages - check +that you have appropriate versions installed. Some also list "|" and offer +a choice of packages. Some of the required packages only exist in the mysociety debian archive (or we have updated versions there), so you will want to add that to your @@ -29,9 +33,6 @@ have updated versions there), so you will want to add that to your deb http://debian.mysociety.org etch main non-free contrib deb-src http://debian.mysociety.org etch main non-free contrib -libgems-ruby1.8 has recently been replaced by rubygems1.8, so you may need to -update the packages file to reflect this, depending on your version. - 2. Configure Database --------------------- diff --git a/app/controllers/admin_request_controller.rb b/app/controllers/admin_request_controller.rb index ff2772b0e..f077691ff 100644 --- a/app/controllers/admin_request_controller.rb +++ b/app/controllers/admin_request_controller.rb @@ -215,10 +215,10 @@ class AdminRequestController < AdminController # Bejeeps, look, sometimes a URL is something that belongs in a controller, jesus. # XXX hammer this square peg into the round MVC hole - should be calling main_url(upload_response_url()) post_redirect = PostRedirect.new( - :uri => upload_response_url(:url_title => info_request.url_title), + :uri => main_url(upload_response_url(:url_title => info_request.url_title, :only_path => true)), :user_id => user.id) post_redirect.save! - url = confirm_url(:email_token => post_redirect.email_token) + url = main_url(confirm_url(:email_token => post_redirect.email_token, :only_path => true)) flash[:notice] = 'Send "' + name + '" <<a href="mailto:' + email + '">' + email + '</a>> this URL: <a href="' + url + '">' + url + "</a> - it will log them in and let them upload a response to this request." redirect_to request_admin_url(info_request) diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb index 29bdb5c88..5a03f0317 100644 --- a/app/controllers/request_controller.rb +++ b/app/controllers/request_controller.rb @@ -650,7 +650,7 @@ class RequestController < ApplicationController if params[:submitted_upload_response] file_name = nil file_content = nil - if params[:file_1].class.to_s == "ActionController::UploadedTempfile" + if !params[:file_1].nil? file_name = params[:file_1].original_filename file_content = params[:file_1].read end diff --git a/app/models/incoming_message.rb b/app/models/incoming_message.rb index 1990af2c5..003499344 100644 --- a/app/models/incoming_message.rb +++ b/app/models/incoming_message.rb @@ -919,9 +919,13 @@ class IncomingMessage < ActiveRecord::Base def get_main_body_text_part leaves = get_attachment_leaves - # Find first part which is text/plain + # Find first part which is text/plain or text/html + # (We have to include HTML, as increasingly there are mail clients that + # include no text alternative for the main part, and we don't want to + # instead use the first text attachment + # e.g. http://www.whatdotheyknow.com/request/list_of_public_authorties) leaves.each do |p| - if p.content_type == 'text/plain' + if p.content_type == 'text/plain' or p.content_type == 'text/html' return p end end diff --git a/app/models/info_request.rb b/app/models/info_request.rb index 8cfc61c4e..6faf0dbad 100644 --- a/app/models/info_request.rb +++ b/app/models/info_request.rb @@ -202,6 +202,8 @@ public # Subject lines for emails about the request def email_subject_request + # XXX pull out this general_register_office specialisation + # into some sort of separate jurisdiction dependent file if self.public_body.url_name == 'general_register_office' # without GQ in the subject, you just get an auto response self.law_used_full + ' request GQ - ' + self.title @@ -209,11 +211,15 @@ public self.law_used_full + ' request - ' + self.title end end - def email_subject_followup - if self.public_body.url_name == 'general_register_office' - 'Re: ' + self.law_used_full + ' request GQ - ' + self.title + def email_subject_followup(incoming_message = nil) + if incoming_message.nil? + 'Re: ' + self.email_subject_request else - 'Re: ' + self.law_used_full + ' request - ' + self.title + if incoming_message.mail.subject.match(/^Re:/i) + incoming_message.mail.subject + else + 'Re: ' + incoming_message.mail.subject + end end end diff --git a/app/models/outgoing_mailer.rb b/app/models/outgoing_mailer.rb index ae7e86f4e..1546d3fd0 100644 --- a/app/models/outgoing_mailer.rb +++ b/app/models/outgoing_mailer.rb @@ -73,7 +73,7 @@ class OutgoingMailer < ApplicationMailer if outgoing_message.what_doing == 'internal_review' return "Internal review of " + info_request.email_subject_request else - return info_request.email_subject_followup + return info_request.email_subject_followup(outgoing_message.incoming_message_followup) end end # Whether we have a valid email address for a followup diff --git a/app/views/help/about.rhtml b/app/views/help/about.rhtml index 0dd540bcb..629e24cf1 100644 --- a/app/views/help/about.rhtml +++ b/app/views/help/about.rhtml @@ -58,6 +58,28 @@ If you like what we're doing, then you can <h1 id="making_requests">Making requests <a href="#making_requests">#</a> </h1> <dl> +<dt id="which_authority">I'm not sure which authority to make my request to, how can I find out? <a href="#which_authority">#</a> </dt> + +<dd> +<p>It can be hard to untangle government's complicated structured, and work out +who knows the information that you want. Here are a few tips: +<ul> +<li>Browse or search WhatDoTheyKnow looking for similar requests to yours.</li> +<li>When you've found an authority you think might have the information, use +the "home page" link on the right hand side of their page to check what they do +on their website.</li> +<li>Contact the authority by phone or email to ask if they hold the kind of +information you're after.</li> +<li>Don't worry excessively about getting the right authority. If you get it +wrong, they ought to advise you who to make the request to instead. +</li> +<li>If you've got a thorny case, please <a href="/help/contact">contact us</a> for help.</li> +</ul> + +</dd> + + + <dt id="missing_body">You're missing the public authority that I want to request from! <a href="#missing_body">#</a> </dt> <dd> @@ -344,7 +366,7 @@ Information Commissioner later about the handling of your request. <ul> <li>Use a different form of your name. The guidance says that "Mr Arthur Thomas Roberts" can make a valid request as "Arthur Roberts", -"A. T. Roberts", or "Mr Roberts", but not as "Arthur" or "A.T.R.". +"A. T. Roberts", or "Mr Roberts", but <strong>not</strong> as "Arthur" or "A.T.R.". </li> <li>Women may use their maiden name.</li> <li>In most cases, you may use any name by which you are "widely known and/or @@ -652,12 +674,14 @@ that authorities resend these with the personal information removed.</p> <dt id="mobiles">Do you publish email addresses or mobile phone numbers? <a href="#mobiles">#</a> </dt> -<dd><p>We automatically remove most emails and mobile numbers from responses to requests. -Please <a href="/help/contact">contact us</a> if we've missed one. -For technical reasons we don't always remove them from attachments, such as some PDFs.</p> +<dd><p>To prevent spam, we automatically remove most emails and some mobile numbers from +responses to requests. Please <a href="/help/contact">contact us</a> if we've +missed one. +For technical reasons we don't always remove them from attachments, such as certain PDFs.</p> <p>If you need to know what an address was that we've removed, please <a - href="/help/contact">get in touch with us</a>. Sometimes they form an important part -of a response. + href="/help/contact">get in touch with us</a>. Occasionally, an email address +forms an important part of a response and we will post it up in an obscured +form in an annotation. </dd> <dt id="copyright"><a name="commercial"></a>What is your policy on copyright of documents?<a href="#copyright">#</a> </dt> @@ -714,7 +738,8 @@ requests, and for good public relations, we'd advise you not to do that. <li> The amazing team of volunteers who run the site, answer your support emails, maintain the database of public authorities and so much more. - Thanks to Tony Bowden, John Cross, Adam McGreggor, Alex Skene, Richard Taylor. + Thanks to Tony Bowden, John Cross, Ben Harris, Adam McGreggor, Alex Skene, + Richard Taylor. </li> <li> Everyone who has helped look up FOI email addresses. diff --git a/app/views/public_body/show.rhtml b/app/views/public_body/show.rhtml index f28581cdd..788d83912 100644 --- a/app/views/public_body/show.rhtml +++ b/app/views/public_body/show.rhtml @@ -5,7 +5,7 @@ <%= render :partial => 'track/tracking_links', :locals => { :track_thing => @track_thing, :own_request => false, :location => 'sidebar' } %> <h2>More about this authority</h2> <% if !@public_body.calculated_home_page.nil? %> - <%= link_to "Home page", @public_body.calculated_home_page %><br> + <%= link_to "Home page of authority", @public_body.calculated_home_page %><br> <% end %> <% if !@public_body.publication_scheme.empty? %> <%= link_to "Publication scheme", @public_body.publication_scheme %><br> diff --git a/config/environment.rb b/config/environment.rb index 9174e0f15..af8f5456f 100644 --- a/config/environment.rb +++ b/config/environment.rb @@ -13,8 +13,8 @@ require File.join(File.dirname(__FILE__), 'boot') # MySociety specific helper functions $:.push(File.join(File.dirname(__FILE__), '../commonlib/rblib')) -# ... if these fail to include, you need the rblib directory from -# mySociety CVS, put it at the same level as the foi directory. +# ... if these fail to include, you need the commonlib submodule from git +# (type "git submodule update --init" in the whatdotheyknow directory) # ruby-ole and ruby-msg. We use a custom ruby-msg to avoid a name conflict $:.unshift(File.join(File.dirname(__FILE__), '../vendor/ruby-ole/lib')) diff --git a/config/packages b/config/packages index be82c6324..b3ed12bfb 100644 --- a/config/packages +++ b/config/packages @@ -5,8 +5,8 @@ rake (>= 0.8.4-1) irb wv poppler-utils -# poppler-utils (>= 0.12.0) -pdftk +# poppler-utils (>= 0.12.0) # this is much better when it is available in Debian stable +pdftk (> 1.41+dfsg-1) | pdftk (< 1.41+dfsg-1) # that version has a non-functionining uncompress option gs-gpl catdoc links @@ -21,7 +21,7 @@ libzip-ruby1.8 libzlib-ruby mahoro-ruby1.8 | libmahoro-ruby1.8 wdg-html-validator -#libapache2-mod-passenger +# libapache2-mod-passenger mutt librack-ruby1.8 (>= 1.0.1-1) librmagick-ruby1.8 diff --git a/spec/controllers/request_controller_spec.rb b/spec/controllers/request_controller_spec.rb index 2d9ff9208..27647e04e 100644 --- a/spec/controllers/request_controller_spec.rb +++ b/spec/controllers/request_controller_spec.rb @@ -1111,6 +1111,92 @@ describe RequestController, "when viewing comments" do end +describe RequestController, "authority uploads a response from the web interface" do + fixtures :info_requests, :info_request_events, :public_bodies, :users + + before(:all) do + # domain after the @ is used for authentication of FOI officers, so to test it + # we need a user which isn't at localhost. + @normal_user = User.new(:name => "Mr. Normal", :email => "normal-user@flourish.org", + :password => PostRedirect.generate_random_token) + @normal_user.save! + + @foi_officer_user = User.new(:name => "The Geraldine Quango", :email => "geraldine-requests@localhost", + :password => PostRedirect.generate_random_token) + @foi_officer_user.save! + end + + it "should require login to view the form to upload" do + @ir = info_requests(:fancy_dog_request) + @ir.public_body.is_foi_officer?(@normal_user).should == false + session[:user_id] = @normal_user.id + + get :upload_response, :url_title => 'why_do_you_have_such_a_fancy_dog' + response.should render_template('user/wrong_user') + end + + it "should let you view upload form if you are an FOI officer" do + @ir = info_requests(:fancy_dog_request) + @ir.public_body.is_foi_officer?(@foi_officer_user).should == true + session[:user_id] = @foi_officer_user.id + + get :upload_response, :url_title => 'why_do_you_have_such_a_fancy_dog' + response.should render_template('request/upload_response') + end + + it "should prevent uploads if you are not a requester" do + @ir = info_requests(:fancy_dog_request) + incoming_before = @ir.incoming_messages.size + session[:user_id] = @normal_user.id + + # post up a photo of the parrot + parrot_upload = fixture_file_upload('parrot.png','image/png') + post :upload_response, :url_title => 'why_do_you_have_such_a_fancy_dog', + :body => "Find attached a picture of a parrot", + :file_1 => parrot_upload, + :submitted_upload_response => 1 + response.should render_template('user/wrong_user') + end + + it "should prevent entirely blank uploads" do + session[:user_id] = @foi_officer_user.id + + post :upload_response, :url_title => 'why_do_you_have_such_a_fancy_dog', :body => "", :submitted_upload_response => 1 + response.should render_template('request/upload_response') + flash[:error].should match(/Please type a message/) + end + + # How do I test a file upload in rails? + # http://stackoverflow.com/questions/1178587/how-do-i-test-a-file-upload-in-rails + it "should let the requester upload a file" do + @ir = info_requests(:fancy_dog_request) + incoming_before = @ir.incoming_messages.size + session[:user_id] = @foi_officer_user.id + + # post up a photo of the parrot + parrot_upload = fixture_file_upload('parrot.png','image/png') + post :upload_response, :url_title => 'why_do_you_have_such_a_fancy_dog', + :body => "Find attached a picture of a parrot", + :file_1 => parrot_upload, + :submitted_upload_response => 1 + + response.should redirect_to(:action => 'show', :url_title => 'why_do_you_have_such_a_fancy_dog') + flash[:notice].should match(/Thank you for responding to this FOI request/) + + # check there is a new attachment + incoming_after = @ir.incoming_messages.size + incoming_after.should == incoming_before + 1 + + # check new attachment looks vaguely OK + new_im = @ir.incoming_messages[-1] + new_im.mail.body.should match(/Find attached a picture of a parrot/) + attachments = new_im.get_attachments_for_display + attachments.size.should == 1 + attachments[0].filename.should == "parrot.png" + attachments[0].display_size.should == "94K" + end +end + diff --git a/spec/fixtures/raw_emails.yml b/spec/fixtures/raw_emails.yml index 4930e9966..d24b61854 100644 --- a/spec/fixtures/raw_emails.yml +++ b/spec/fixtures/raw_emails.yml @@ -6,7 +6,7 @@ useless_raw_email: Date: Tue, 13 Nov 2007 11:39:55 +0000 NoCc: foi+request-1-4b571715@cat Bcc: - Subject: Re: Freedom of Information Request - Why do you have such a fancy dog? + Subject: Geraldine FOI Code AZXB421 Reply-To: In-Reply-To: <471f1eae5d1cb_7347..fdbe67386163@cat.tmail> diff --git a/spec/models/incoming_message_spec.rb b/spec/models/incoming_message_spec.rb index cf8db63cb..cf51c8a42 100644 --- a/spec/models/incoming_message_spec.rb +++ b/spec/models/incoming_message_spec.rb @@ -157,6 +157,9 @@ describe IncomingMessage, " when censoring data" do data.should == "His email was x\000x\000x\000@\000x\000x\000x\000.\000x\000x\000x\000, indeed" end + # As at March 9th 2010: This test fails with pdftk 1.41+dfsg-1 installed + # which is in Ubuntu Karmic. It works again for the lasest version + # 1.41+dfsg-7 in Debian unstable. And it works for Debian stable. it "should replace everything in PDF files" do orig_pdf = load_file_fixture('tfl.pdf') pdf = orig_pdf.dup diff --git a/spec/models/outgoing_mailer_spec.rb b/spec/models/outgoing_mailer_spec.rb index de59b09b2..0bbe39cad 100644 --- a/spec/models/outgoing_mailer_spec.rb +++ b/spec/models/outgoing_mailer_spec.rb @@ -66,4 +66,51 @@ 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 + + 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] + + ir.email_subject_request.should == "Freedom of Information request - Why do you have & such a fancy dog?" + end + + it "should use 'Re:' and inital request subject for followups which aren't replies to particular messages" do + ir = info_requests(:fancy_dog_request) + om = outgoing_messages(:useless_outgoing_message) + + OutgoingMailer.subject_for_followup(ir, om).should == "Re: Freedom of Information request - Why do you have & such a fancy dog?" + end + + it "should prefix with Re: the subject of the message being replied to" do + ir = info_requests(:fancy_dog_request) + im = ir.incoming_messages[0] + om = outgoing_messages(:useless_outgoing_message) + om.incoming_message_followup = im + + OutgoingMailer.subject_for_followup(ir, om).should == "Re: Geraldine FOI Code AZXB421" + end + + it "should not add Re: prefix if there already is such a prefix" do + ir = info_requests(:fancy_dog_request) + im = ir.incoming_messages[0] + om = outgoing_messages(:useless_outgoing_message) + om.incoming_message_followup = im + + im.raw_email.data = im.raw_email.data.sub("Subject: Geraldine FOI Code AZXB421", "Subject: Re: Geraldine FOI Code AZXB421") + OutgoingMailer.subject_for_followup(ir, om).should == "Re: Geraldine FOI Code AZXB421" + end + + 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] + om = outgoing_messages(:useless_outgoing_message) + om.incoming_message_followup = im + + im.raw_email.data = im.raw_email.data.sub("Subject: Geraldine FOI Code AZXB421", "Subject: re: Geraldine FOI Code AZXB421") + OutgoingMailer.subject_for_followup(ir, om).should == "re: Geraldine FOI Code AZXB421" + end +end + diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 72457815a..ddb9ab14b 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -7,7 +7,7 @@ require 'spec/rails' Spec::Runner.configure do |config| config.use_transactional_fixtures = true config.use_instantiated_fixtures = false - config.fixture_path = RAILS_ROOT + '/spec/fixtures' + config.fixture_path = RAILS_ROOT + '/spec/fixtures/' # You can declare fixtures for each behaviour like this: # describe "...." do @@ -1,11 +1,7 @@ Next (things that will reduce admin time mainly) ==== -Should really make replies munge subject of last response, rather than start -afresh with subject - authorities use FOI code in subject as here: -http://www.whatdotheyknow.com/request/causes_of_the_financial_crisis#incoming-12779 -* write some tests first, of every kind of followup -* grep for subject_for_followup and email_subject_request +Make it so when you make followups the whole request is shown on the page. Finish profile photo @@ -96,6 +92,10 @@ Remove comments visible Later ===== +Show the Subject: lines on request pages. Perhaps only show them where they are +substantively different (modulo Re: and Fwd:) from the title and other subjects +- so you can see any FOI code number the authority has put in the subject. + For Scotland, don't need to say "normally" equivocally when it is taking more than 20 days (as there is no public interest test). @@ -215,7 +215,6 @@ Help page improvements: Show similar requests after you have filed yours - maybe on preview too. -Test code for FOI officer upload Test code for rendering lots of different attachments and filetypes Test code for internal review submitting @@ -389,6 +388,13 @@ Display pasted quotes in annotations better: Totally new features -------------------- +Scribd embedded version of documents + +Read reply - ask for exchange read receipts, and show if mail was read. + +Telephone numbers. Add advice in workflow to call authority first to check +form they have info in. Store telephone numbers in database. + Give authorities interface for editing their request email address and resend messages to them |