diff options
| -rw-r--r-- | app/controllers/admin_public_body_controller.rb | 6 | ||||
| -rw-r--r-- | app/models/info_request.rb | 23 | ||||
| -rw-r--r-- | app/models/public_body.rb | 8 | ||||
| -rw-r--r-- | app/models/request_mailer.rb | 53 | ||||
| -rw-r--r-- | app/models/user_info_request_sent_alert.rb | 5 | ||||
| -rw-r--r-- | app/views/general/search.rhtml | 10 | ||||
| -rw-r--r-- | app/views/help/about.rhtml | 2 | ||||
| -rw-r--r-- | app/views/request/_followup.rhtml | 10 | ||||
| -rw-r--r-- | app/views/request/show_response.rhtml | 2 | ||||
| -rw-r--r-- | app/views/request_mailer/not_clarified_alert.rhtml | 9 | ||||
| -rw-r--r-- | config/crontab.ugly | 3 | ||||
| -rwxr-xr-x | script/alert-not-clarified-request | 7 | ||||
| -rw-r--r-- | spec/controllers/request_controller_spec.rb | 35 | ||||
| -rw-r--r-- | todo.txt | 43 |
14 files changed, 174 insertions, 42 deletions
diff --git a/app/controllers/admin_public_body_controller.rb b/app/controllers/admin_public_body_controller.rb index 071af1ee8..e8ee58f95 100644 --- a/app/controllers/admin_public_body_controller.rb +++ b/app/controllers/admin_public_body_controller.rb @@ -4,7 +4,7 @@ # Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved. # Email: francis@mysociety.org; WWW: http://www.mysociety.org/ # -# $Id: admin_public_body_controller.rb,v 1.13 2008-04-15 01:06:12 francis Exp $ +# $Id: admin_public_body_controller.rb,v 1.14 2008-05-21 22:37:32 francis Exp $ class AdminPublicBodyController < ApplicationController layout "admin" @@ -71,13 +71,13 @@ class AdminPublicBodyController < ApplicationController if not params[:tag].empty? # Try with dry run first csv_contents = params[:csv_file].read - en = PublicBody.import_csv(csv_contents, params[:tag], true) + en = PublicBody.import_csv(csv_contents, params[:tag], true, admin_http_auth_user()) errors = en[0] notes = en[1] if errors.size == 0 # And if OK, with real run - en = PublicBody.import_csv(csv_contents, params[:tag], false) + en = PublicBody.import_csv(csv_contents, params[:tag], false, admin_http_auth_user()) errors = en[0] notes = en[1] if errors.size != 0 diff --git a/app/models/info_request.rb b/app/models/info_request.rb index ece806009..14cd503ba 100644 --- a/app/models/info_request.rb +++ b/app/models/info_request.rb @@ -22,7 +22,7 @@ # Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved. # Email: francis@mysociety.org; WWW: http://www.mysociety.org/ # -# $Id: info_request.rb,v 1.113 2008-05-21 10:51:24 francis Exp $ +# $Id: info_request.rb,v 1.114 2008-05-21 22:37:33 francis Exp $ require 'digest/sha1' require File.join(File.dirname(__FILE__),'../../vendor/plugins/acts_as_xapian/lib/acts_as_xapian') @@ -262,7 +262,7 @@ public # -- sent at all # -- OR the same message was resent # -- OR the public body requested clarification, and a follow up was sent - def last_outgoing_message_forming_initial_request + def last_event_forming_initial_request last_sent = nil expecting_clarification = false for event in self.info_request_events @@ -271,14 +271,12 @@ public end if [ 'sent', 'resent', 'followup_sent' ].include?(event.event_type) - outgoing_message = event.outgoing_message - if last_sent.nil? - last_sent = outgoing_message + last_sent = event elsif event.event_type == 'resent' - last_sent = outgoing_message + last_sent = event elsif expecting_clarification and event.event_type == 'followup_sent' - last_sent = outgoing_message + last_sent = event expecting_clarification = false end end @@ -297,12 +295,15 @@ public # # Freedom of Information Act 2000 section 10 # - # XXX how do we cope with case where extra info was required from the requester - # by the public body in order to fulfill the request, as per sections 1(3) and 10(6b) ? + # How do we cope with case where extra info was required from the requester + # by the public body in order to fulfill the request, as per sections 1(3) + # and 10(6b) ? For clarifications this is covered by + # last_event_forming_initial_request. There may be more obscure + # things, e.g. fees, not properly covered. def date_response_required_by # Find the ear - last_sent = self.last_outgoing_message_forming_initial_request - last_sent_at = last_sent.last_sent_at + last_sent = self.last_event_forming_initial_request + last_sent_at = last_sent.outgoing_message.last_sent_at # Count forward 20 working days days_passed = 0 diff --git a/app/models/public_body.rb b/app/models/public_body.rb index 03e74f2b7..9b7c97c4c 100644 --- a/app/models/public_body.rb +++ b/app/models/public_body.rb @@ -21,7 +21,7 @@ # Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved. # Email: francis@mysociety.org; WWW: http://www.mysociety.org/ # -# $Id: public_body.rb,v 1.71 2008-05-21 10:51:24 francis Exp $ +# $Id: public_body.rb,v 1.72 2008-05-21 22:37:33 francis Exp $ require 'csv' require 'set' @@ -154,7 +154,7 @@ class PublicBody < ActiveRecord::Base # Import from CSV. Just tests things and returns messages if dry_run is true. # Returns an array of [array of errors, array of notes]. If there are errors, # always rolls back (as with dry_run). - def self.import_csv(csv, tag, dry_run = false) + def self.import_csv(csv, tag, dry_run, editor) errors = [] notes = [] @@ -195,14 +195,14 @@ class PublicBody < ActiveRecord::Base if public_body.request_email != email notes.push "line " + line.to_s + ": updating email for '" + name + "' from " + public_body.request_email + " to " + email public_body.request_email = email - public_body.last_edit_editor = 'import_csv' + public_body.last_edit_editor = editor public_body.last_edit_comment = 'Updated from spreadsheet' public_body.save! end else # New public body notes.push "line " + line.to_s + ": new authority '" + name + "' with email " + email - public_body = PublicBody.new(:name => name, :request_email => email, :short_name => "", :last_edit_editor => "import_csv", :last_edit_comment => 'Created from spreadsheet') + public_body = PublicBody.new(:name => name, :request_email => email, :short_name => "", :last_edit_editor => editor, :last_edit_comment => 'Created from spreadsheet') public_body.tag_string = tag public_body.save! end diff --git a/app/models/request_mailer.rb b/app/models/request_mailer.rb index cb43b59f8..cfe837b3b 100644 --- a/app/models/request_mailer.rb +++ b/app/models/request_mailer.rb @@ -4,7 +4,7 @@ # Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved. # Email: francis@mysociety.org; WWW: http://www.mysociety.org/ # -# $Id: request_mailer.rb,v 1.32 2008-05-19 12:01:22 francis Exp $ +# $Id: request_mailer.rb,v 1.33 2008-05-21 22:37:33 francis Exp $ class RequestMailer < ApplicationMailer @@ -113,6 +113,23 @@ class RequestMailer < ApplicationMailer @body = { :incoming_message => incoming_message, :info_request => info_request, :url => url } end + # Tell the requester that they need to clarify their request + def not_clarified_alert(info_request, incoming_message) + respond_url = show_response_url(:id => info_request.id, :incoming_message_id => incoming_message.id) + respond_url = respond_url + "#show_response_followup" + + post_redirect = PostRedirect.new( + :uri => respond_url, + :user_id => info_request.user.id) + post_redirect.save! + url = confirm_url(:email_token => post_redirect.email_token) + + @from = contact_from_name_and_email + @recipients = info_request.user.name_and_email + @subject = "Clarify your FOI request - " + info_request.title + @body = { :incoming_message => incoming_message, :info_request => info_request, :url => url } + end + # Class function, called by script/mailin with all incoming responses. # [ This is a copy (Monkeypatch!) of function from action_mailer/base.rb, @@ -150,17 +167,19 @@ class RequestMailer < ApplicationMailer #STDERR.puts "alert_overdue_requests" info_requests = InfoRequest.find(:all, :conditions => [ "described_state = 'waiting_response' and not awaiting_description" ], :include => [ :user ] ) for info_request in info_requests + alert_event_id = info_request.last_event_forming_initial_request.id # Only overdue requests if info_request.calculate_status == 'waiting_response_overdue' # For now, just to the user who created the request - sent_already = UserInfoRequestSentAlert.find(:first, :conditions => [ "alert_type = 'overdue_1' and user_id = ? and info_request_id = ?", info_request.user_id, info_request.id]) + sent_already = UserInfoRequestSentAlert.find(:first, :conditions => [ "alert_type = 'overdue_1' and user_id = ? and info_request_id = ? and info_request_event_id = ?", info_request.user_id, info_request.id, alert_event_id]) if sent_already.nil? # Alert not yet sent for this user - #STDERR.puts "sending overdue alert to info_request " + info_request.id.to_s + " user " + info_request.user_id.to_s + #STDERR.puts "sending overdue alert to info_request " + info_request.id.to_s + " user " + info_request.user_id.to_s + " event " + alert_event_id store_sent = UserInfoRequestSentAlert.new store_sent.info_request = info_request store_sent.user = info_request.user store_sent.alert_type = 'overdue_1' + store_sent.info_request_event_id = alert_event_id RequestMailer.deliver_overdue_alert(info_request, info_request.user) store_sent.save! #STDERR.puts "sent " + info_request.user.email @@ -197,6 +216,34 @@ class RequestMailer < ApplicationMailer end end + # Send email alerts for requests which need clarification. Goes out 3 days + # after last update of event. + def self.alert_not_clarified_request() + #STDERR.puts "alert_not_clarified_request" + info_requests = InfoRequest.find(:all, :conditions => [ "not awaiting_description and described_state = 'waiting_clarification' and info_requests.updated_at < ?", Time.now() - 3.days ], :include => [ :user ], :order => "info_requests.id" ) + for info_request in info_requests + alert_event_id = info_request.get_last_response_event_id + last_response_message = info_request.get_last_response + if alert_event_id.nil? + raise "internal error, no last response while making alert not clarified reminder, request id " + info_request.id.to_s + end + # To the user who created the request + sent_already = UserInfoRequestSentAlert.find(:first, :conditions => [ "alert_type = 'not_clarified_1' and user_id = ? and info_request_id = ? and info_request_event_id = ?", info_request.user_id, info_request.id, alert_event_id]) + if sent_already.nil? + # Alert not yet sent for this user + STDERR.puts "sending clarification reminder alert to info_request " + info_request.id.to_s + " user " + info_request.user_id.to_s + " event " + alert_event_id.to_s + store_sent = UserInfoRequestSentAlert.new + store_sent.info_request = info_request + store_sent.user = info_request.user + store_sent.alert_type = 'not_clarified_1' + store_sent.info_request_event_id = alert_event_id + RequestMailer.deliver_not_clarified_alert(info_request, last_response_message) + store_sent.save! + #STDERR.puts "sent " + info_request.user.email + end + end + end + end diff --git a/app/models/user_info_request_sent_alert.rb b/app/models/user_info_request_sent_alert.rb index 872286c88..2b6cfed1d 100644 --- a/app/models/user_info_request_sent_alert.rb +++ b/app/models/user_info_request_sent_alert.rb @@ -17,7 +17,7 @@ # Copyright (c) 2008 UK Citizens Online Democracy. All rights reserved. # Email: francis@mysociety.org; WWW: http://www.mysociety.org/ # -# $Id: user_info_request_sent_alert.rb,v 1.15 2008-05-21 10:51:24 francis Exp $ +# $Id: user_info_request_sent_alert.rb,v 1.16 2008-05-21 22:37:33 francis Exp $ class UserInfoRequestSentAlert < ActiveRecord::Base belongs_to :user @@ -25,7 +25,8 @@ class UserInfoRequestSentAlert < ActiveRecord::Base validates_inclusion_of :alert_type, :in => [ 'overdue_1', # tell user that info request has become overdue - 'new_response_reminder_1' # reminder user to classify the recent response + 'new_response_reminder_1', # reminder user to classify the recent response + 'not_clarified_1', # reminder that user has to explain part of the request ] end diff --git a/app/views/general/search.rhtml b/app/views/general/search.rhtml index 60c268f86..85afe7846 100644 --- a/app/views/general/search.rhtml +++ b/app/views/general/search.rhtml @@ -1,5 +1,13 @@ <% @show_tips = @xapian_requests.nil? || (@total_hits == 0) %> +<% # XXX should call find_tracking_people in controller, not here / in partial + if @track_thing and TrackThing.find_tracking_people(@track_thing).size > 0 %> + <div id="request_sidebar"> + <h2>People tracking this search</h2> + <%= render :partial => 'track/tracking_people_and_link', :locals => { :track_thing => @track_thing, :list_people => true } %> + </div> +<% end %> + <% if @query.nil? %> <% @title = "Search Freedom of Information requests, public authorities and users" %> <h1><%=@title%></h1> @@ -39,7 +47,7 @@ <% end %> <% if @track_thing %> - <%= render :partial => 'track/tracking_people_and_link', :locals => { :track_thing => @track_thing, :own_request => false, :list_people => false } %> + <%= render :partial => 'track/tracking_people_and_link', :locals => { :track_thing => @track_thing, :list_people => false } %> <% end %> <% if @xapian_bodies.results.size > 0 %> diff --git a/app/views/help/about.rhtml b/app/views/help/about.rhtml index 71890380e..07f88ad11 100644 --- a/app/views/help/about.rhtml +++ b/app/views/help/about.rhtml @@ -65,6 +65,8 @@ primary care trusts or all schools, then please any UK bank holidays. The date that the response is due by is shown on the page for your request. You will be emailed if this date goes by without a response, so you can send the public authority another note to hurry them up. +<strong>Note:</strong> If you had to clarify your request, the clock starts from that date, +instead of the date they received your initial request. </dd> <dt id="not_satifised">What if I'm not satisfied with the response?</dt> diff --git a/app/views/request/_followup.rhtml b/app/views/request/_followup.rhtml index 0a9df60c2..fe7b09ea5 100644 --- a/app/views/request/_followup.rhtml +++ b/app/views/request/_followup.rhtml @@ -20,6 +20,16 @@ <p>Use this to tell the public authority something, such as to clarify your request, or if they are late responding.</p> + <% if @info_request.calculate_status == 'waiting_response_overdue' %> + <p>This request is currently <strong>overdue a response</strong> from <%= + public_body_link(@info_request.public_body) %>. + The response was due + on <strong><%= simple_date(@info_request.date_response_required_by) %></strong> + (<%= link_to "more info", about_url + "#quickly_response" %>). You can say + that under the Freedom of Information Act they should have replied by now. + </p> + <% end %> + <% form_for(:outgoing_message, @outgoing_message) do |o| %> <p> <%= o.text_area :body, :rows => 10, :cols => 55 %> diff --git a/app/views/request/show_response.rhtml b/app/views/request/show_response.rhtml index 77d03214d..45c3f02d4 100644 --- a/app/views/request/show_response.rhtml +++ b/app/views/request/show_response.rhtml @@ -26,7 +26,7 @@ <h2>Response to FOI request '<%= request_link @info_request %>'</h2> <% end %> <% end %> - + <% if @incoming_message.nil? %> <%= render :partial => 'correspondence', :locals => { :info_request_event => @info_request.get_last_outgoing_event, :incoming_message => nil } %> <% else %> diff --git a/app/views/request_mailer/not_clarified_alert.rhtml b/app/views/request_mailer/not_clarified_alert.rhtml new file mode 100644 index 000000000..8752d0606 --- /dev/null +++ b/app/views/request_mailer/not_clarified_alert.rhtml @@ -0,0 +1,9 @@ +<%=@info_request.public_body.name%> has asked you to explain +part of your FOI request. To do this, click on the link below. + +<%=@url%> + +You will only get an answer to your request if you follow up +with the clarification. + +-- the WhatDoTheyKnow team diff --git a/config/crontab.ugly b/config/crontab.ugly index 0f2054fbd..b1016fe97 100644 --- a/config/crontab.ugly +++ b/config/crontab.ugly @@ -4,7 +4,7 @@ # Copyright (c) 2008 UK Citizens Online Democracy. All rights reserved. # Email: francis@mysociety.org. WWW: http://www.mysociety.org/ # -# $Id: crontab.ugly,v 1.11 2008-05-09 01:00:42 francis Exp $ +# $Id: crontab.ugly,v 1.12 2008-05-21 22:37:34 francis Exp $ PATH=/usr/local/bin:/usr/bin:/bin MAILTO=team@whatdotheyknow.com @@ -19,6 +19,7 @@ MAILTO=team@whatdotheyknow.com # Once a day, early morning 23 4 * * * !!(*= $user *)!! run-with-lockfile -n /data/vhost/!!(*= $vhost *)!!/delete-old-sessions.lock /data/vhost/!!(*= $vhost *)!!/mysociety/foi/script/delete-old-sessions || echo "stalled?" 0 8 * * * !!(*= $user *)!! run-with-lockfile -n /data/vhost/!!(*= $vhost *)!!/alert-new-response-reminders.lock /data/vhost/!!(*= $vhost *)!!/mysociety/foi/script/alert-new-response-reminders || echo "stalled?" +0 8 * * * !!(*= $user *)!! run-with-lockfile -n /data/vhost/!!(*= $vhost *)!!/alert-not-clarified-request.lock /data/vhost/!!(*= $vhost *)!!/mysociety/foi/script/alert-not-clarified-request || echo "stalled?" # Once a day on all servers 43 2 * * * !!(*= $user *)!! /data/vhost/!!(*= $vhost *)!!/mysociety/foi/script/request-creation-graph diff --git a/script/alert-not-clarified-request b/script/alert-not-clarified-request new file mode 100755 index 000000000..ac6636dce --- /dev/null +++ b/script/alert-not-clarified-request @@ -0,0 +1,7 @@ +#!/bin/bash + +LOC=`dirname $0` + +$LOC/runner 'RequestMailer.alert_not_clarified_request' + + diff --git a/spec/controllers/request_controller_spec.rb b/spec/controllers/request_controller_spec.rb index d7a199a96..cd3160ddb 100644 --- a/spec/controllers/request_controller_spec.rb +++ b/spec/controllers/request_controller_spec.rb @@ -342,6 +342,41 @@ describe RequestController, "sending unclassified new response reminder alerts" end +describe RequestController, "clarification required alerts" do + integrate_views + fixtures :info_requests, :info_request_events, :public_bodies, :users, :incoming_messages, :outgoing_messages # all needed as integrating views + + it "should send an alert" do + ir = info_requests(:fancy_dog_request) + ir.set_described_state('waiting_clarification') + # this is pretty horrid, but will do :) need to make it waiting + # clarification more than 3 days ago for the alerts to go out. + ActiveRecord::Base.connection.update "update info_requests set updated_at = now() - '5 day'::interval where id = " + ir.id.to_s + ir.reload + + RequestMailer.alert_not_clarified_request + + deliveries = ActionMailer::Base.deliveries + deliveries.size.should == 1 + mail = deliveries[0] + mail.body.should =~ /asked you to explain/ + mail.to_addrs.to_s.should == info_requests(:fancy_dog_request).user.name_and_email + mail.body =~ /(http:\/\/.*\/c\/(.*))/ + mail_url = $1 + mail_token = $2 + + session[:user_id].should be_nil + controller.test_code_redirect_by_email_token(mail_token, self) # XXX hack to avoid having to call User controller for email link + session[:user_id].should == info_requests(:fancy_dog_request).user.id + + response.should render_template('show_response') + assigns[:info_request].should == info_requests(:fancy_dog_request) + end + +end + + + @@ -1,6 +1,30 @@ +CVS +--- +Show people tracking same query, when you are on search page. +Put name of admin user rather than import_csv. +Send automated email to remind people to clarify their request. +Send response overdue alerts multiple times for one request. +Overdue response alert email click through should show how many days overdue +it is near where you write your reply. + + + +Altering updated_at for test code + +Test: Count how many clarification emails need to go out and run them manually +after deploying + +XXX - is 3 day thing in self.alert_new_response_reminders stopping it working completely +Change 14 days to 7? for when manually choose responses +Check the table to see which of those alerts have gone out recently + Test: Fix bug as to why "Vacant Council Housing Stock" didn't appear in my email alert - i.e. created at date compared, not described at date. +#links and ?post_redirect knackered - e.g. from overdue email + +git push + FOI requests to use to test it ============================== @@ -42,13 +66,11 @@ Or just look at referrers as Julian says Advertise WDTK search queries on TWFY Advertise alerts on end pages with WDTK -Do some performance profiling +user/show.rhtml sidebar vs. generic sidebar? Later ===== -Show people tracking same query, when you are on search page - Display current page (when not on first page), or perhaps range on both the /list pages, and on the /..../similar pages (maybe I missed it elsewhere too). Should make a general will_paginate like function to do this part, and call @@ -64,8 +86,6 @@ Never update cached attachment text unless cache is explicitly cleared Make highlighting of search terms in RSS actually light up (maybe ask Tommy) -Put name of admin user rather than import_csv - Link more clearly to full request and next/previous correspondence (ask Matthew) Perhaps always redirect to internal # links from the /response/ pages e.g. http://www.flickr.com/groups/central/discuss/72157604494352123/72157604515433851/ @@ -77,8 +97,6 @@ When you click RSS feed when on own request, default to RSS in the choice on nex Offer search on 404s -Send automated email to remind people to clarify their request - Preview when sending followups - especially people need to see quoting/subject when sending "my response is late" @@ -87,6 +105,7 @@ Interface for when you change your email address - easier to do now with post_re One of the PDFs on live site has: Error: PDF version 1.6 -- xpdf supports version 1.5 (continuing anyway) Need to upgrade to poppler-utils? +Check that it now shows editor fine Merge workflow into one stream - find information, if you can't find it then request it. so just search is on front page, with popular stuff below a la Google Directory @@ -95,17 +114,11 @@ Use spelling correction for public bodies search? In sidebar of request Share this request on Facebook, by email etc. Email icon here: http://www.guardian.co.uk/news/video/2008/apr/03/mugabe + Simple Digg style "I lke this request" button (doesn't tracking do?) Tell application developer if working days table not up to date, and needs updating -Response overdue alerts (NOT TRACKS - this is old style alerts to requester) -are only sent first time a request goes into that state :( Store id in alert of -last event which resets the due date - -Overdue response alert email click through should show how many days overdue - it is near where you write your reply - Synthesise these tips into our handful of snappy snappy bullet points http://community.foe.co.uk/tools/right_to_know/tips.html http://www.justice.gov.uk/requestinginformation.htm @@ -178,8 +191,6 @@ Add geographical location to council import Holding pen with comments - new requests don't get sent straight away, but are delayed while people help improve them. -Simple Digg style "I lke this request" button (doesn't tracking do?) - Editable user profile, including photo upload Show documents inline (word docs etc.) |