aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rwxr-xr-xINSTALL.txt11
-rw-r--r--app/controllers/admin_request_controller.rb4
-rw-r--r--app/controllers/request_controller.rb2
-rw-r--r--app/models/incoming_message.rb8
-rw-r--r--app/models/info_request.rb14
-rw-r--r--app/models/outgoing_mailer.rb2
-rw-r--r--app/views/help/about.rhtml39
-rw-r--r--app/views/public_body/show.rhtml2
-rw-r--r--config/environment.rb4
-rw-r--r--config/packages6
-rw-r--r--spec/controllers/request_controller_spec.rb86
-rw-r--r--spec/fixtures/raw_emails.yml2
-rw-r--r--spec/models/incoming_message_spec.rb3
-rw-r--r--spec/models/outgoing_mailer_spec.rb47
-rw-r--r--spec/spec_helper.rb2
-rw-r--r--todo.txt18
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 + '" &lt;<a href="mailto:' + email + '">' + email + '</a>&gt; 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
diff --git a/todo.txt b/todo.txt
index 66b807c92..4141bd73a 100644
--- a/todo.txt
+++ b/todo.txt
@@ -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