When a response is overdue, email the request creator to tell them so they can send a followup.

15 files changed, 190 insertions, 28 deletions

diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb
index 561c95b59..c669f0aaf 100644
--- a/app/controllers/request_controller.rb
+++ b/app/controllers/request_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: request_controller.rb,v 1.56 2008-02-21 20:10:21 francis Exp $
+# $Id: request_controller.rb,v 1.57 2008-02-22 01:58:36 francis Exp $
 
 class RequestController < ApplicationController
     
@@ -193,8 +193,12 @@ class RequestController < ApplicationController
 
     # Show an individual incoming message, and allow followup
     def show_response
-        @incoming_message = IncomingMessage.find(params[:incoming_message_id])
-        @info_request = @incoming_message.info_request
+        if params[:incoming_message_id].nil?
+            @incoming_message = nil
+        else
+            @incoming_message = IncomingMessage.find(params[:incoming_message_id])
+        end
+        @info_request = InfoRequest.find(params[:id].to_i)
         @collapse_quotes = params[:unfold] ? false : true
         @is_owning_user = !authenticated_user.nil? && authenticated_user.id == @info_request.user_id
 
@@ -209,8 +213,8 @@ class RequestController < ApplicationController
         })
         @outgoing_message = OutgoingMessage.new(params_outgoing_message)
 
-        if @incoming_message.info_request_id != params[:id].to_i
-            raise sprintf("Incoming message %d does not belong to request %d", @incoming_message.info_request_id, params[:id])
+        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)
         end
 
         if !params[:submitted_followup].nil?
diff --git a/app/models/info_request.rb b/app/models/info_request.rb
index bad751efe..48ccedcc1 100644
--- a/app/models/info_request.rb
+++ b/app/models/info_request.rb
@@ -20,7 +20,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.44 2008-02-21 20:45:51 francis Exp $
+# $Id: info_request.rb,v 1.45 2008-02-22 01:58:36 francis Exp $
 
 require 'digest/sha1'
 
@@ -36,6 +36,7 @@ class InfoRequest < ActiveRecord::Base
     has_many :outgoing_messages
     has_many :incoming_messages
     has_many :info_request_events
+    has_many :user_info_request_sent_alerts
 
     belongs_to :dsecribed_last_incoming_message_id
 
@@ -280,6 +281,17 @@ public
         return nil
     end
 
+    # The last outgoing message
+    def get_last_outgoing_event
+        events = self.info_request_events.find(:all, :order => "created_at")
+        events.reverse.each do |e|
+            if e.event_type == 'sent' || e.event_type == 'resent' || e.event_type == 'followup_sent'
+                return e
+            end
+        end
+        return nil
+    end
+
     # Text from the the initial request, for use in summary display
     def initial_request_text
         if outgoing_messages.empty? # mainly for use with incomplete fixtures
diff --git a/app/models/request_mailer.rb b/app/models/request_mailer.rb
index 515008209..20ac78b70 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.22 2008-02-21 20:10:21 francis Exp $
+# $Id: request_mailer.rb,v 1.23 2008-02-22 01:58:36 francis Exp $
 
 class RequestMailer < ApplicationMailer
     
@@ -30,7 +30,11 @@ class RequestMailer < ApplicationMailer
         @from = info_request.incoming_name_and_email
         headers 'Sender' => info_request.envelope_name_and_email,
                 'Reply-To' => @from
-        @recipients = incoming_message_followup.mail.from_addrs.to_s
+        if incoming_message_followup.nil?
+            @recipients = info_request.recipient_name_and_email
+        else
+            @recipients = incoming_message_followup.mail.from_addrs.to_s
+        end
         @subject    = 'Re: Freedom of Information Request - ' + info_request.title
         @body       = {:info_request => info_request, :outgoing_message => outgoing_message,
             :incoming_message_followup => incoming_message_followup,
@@ -70,6 +74,29 @@ class RequestMailer < ApplicationMailer
         @body = { :incoming_message => incoming_message, :info_request => info_request, :url => url }
     end
 
+    # Tell the requester that a new response has arrived
+    def overdue_alert(info_request, user)
+        last_response = info_request.get_last_response
+        if last_response.nil?
+            respond_url = show_response_no_followup_url(:id => info_request.id)
+        else
+            respond_url = show_response_url(:id => info_request.id, :incoming_message_id => last_response.id)
+        end
+        respond_url = respond_url + "#show_response_followup" 
+
+        post_redirect = PostRedirect.new(
+            :uri => respond_url,
+            :user_id => user.id)
+        post_redirect.save!
+        url = confirm_url(:email_token => post_redirect.email_token)
+
+        @from = contact_from_name_and_email
+        @recipients = user.name_and_email
+        @subject = "You're overdue a response to your FOI request - " + info_request.title
+        @body = { :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,
     # but which additionally passes the raw_email to the member function, as we
@@ -107,4 +134,30 @@ class RequestMailer < ApplicationMailer
         end
     end
 
+    # Send email alerts for overdue requests
+    def self.alert_overdue_requests()
+        #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
+            # 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])
+                if sent_already.nil?
+                    # Alert not yet sent for this user
+                    puts "sending overdue alert to info_request " + info_request.id.to_s + " user " + info_request.user_id.to_s
+                    store_sent = UserInfoRequestSentAlert.new
+                    store_sent.info_request = info_request
+                    store_sent.user = info_request.user
+                    store_sent.alert_type = 'overdue_1'
+                    RequestMailer.deliver_overdue_alert(info_request, info_request.user)
+                    store_sent.save!
+                    #puts "sent " + info_request.user.email
+                end
+            end
+        end
+
+    end
 end
+
+
diff --git a/app/models/user.rb b/app/models/user.rb
index b7897bcba..1fa71dffb 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -19,7 +19,7 @@
 # Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved.
 # Email: francis@mysociety.org; WWW: http://www.mysociety.org/
 #
-# $Id: user.rb,v 1.27 2008-02-21 20:45:51 francis Exp $
+# $Id: user.rb,v 1.28 2008-02-22 01:58:36 francis Exp $
 
 require 'digest/sha1'
 
@@ -31,6 +31,7 @@ class User < ActiveRecord::Base
     validates_presence_of :hashed_password, :message => "^Please enter a password"
 
     has_many :info_requests
+    has_many :user_info_request_sent_alerts
 
     attr_accessor :password_confirmation
     validates_confirmation_of :password, :message =>"^Please enter the same password twice"
diff --git a/app/models/user_info_request_sent_alert.rb b/app/models/user_info_request_sent_alert.rb
new file mode 100644
index 000000000..8a272d274
--- /dev/null
+++ b/app/models/user_info_request_sent_alert.rb
@@ -0,0 +1,19 @@
+# models/user_info_request_sent_alert.rb:
+# Whether an alert has been sent to this user for this info_request, for a
+# given type of alert.
+#
+# 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.1 2008-02-22 01:58:36 francis Exp $
+
+class UserInfoRequestSentAlert < ActiveRecord::Base
+    belongs_to :user
+    belongs_to :info_request
+
+    validates_inclusion_of :alert_type, :in => [ 
+        'overdue_1' # tell user that info request has become overdue
+    ]
+end
+
+
diff --git a/app/views/request/_correspondence.rhtml b/app/views/request/_correspondence.rhtml
index d8b8887a8..336f909e2 100644
--- a/app/views/request/_correspondence.rhtml
+++ b/app/views/request/_correspondence.rhtml
@@ -45,7 +45,7 @@
             <% elsif outgoing_message.message_type == 'followup' %>
                 <% if outgoing_message.status == 'sent' %>
                     wrote to
-                    <% if !outgoing_message.incoming_message_followup.safe_mail_from.nil? %>
+                    <% if !outgoing_message.incoming_message_followup.nil? && !outgoing_message.incoming_message_followup.safe_mail_from.nil? %>
                         <%= outgoing_message.incoming_message_followup.safe_mail_from %> of
                     <% end %>
                     <%= public_body_link(@info_request.public_body) %>
@@ -56,7 +56,8 @@
                 <% end %> 
             <% else raise "unknown outgoing_message.message_type" %>
             <% end %>
-        </p>
+            (<%= link_to "send follow up", show_response_no_followup_url(:id => outgoing_message.info_request.id, :incoming_message_id => nil) + "#show_response_followup" %>)
+    </p>
     <% elsif info_request_event.event_type == 'resent' %>
         <p class="event_plain"> 
             Sent to <%= public_body_link(@info_request.public_body) %>
diff --git a/app/views/request/_followup.rhtml b/app/views/request/_followup.rhtml
index 3f6f5322d..a84da3214 100644
--- a/app/views/request/_followup.rhtml
+++ b/app/views/request/_followup.rhtml
@@ -1,13 +1,19 @@
 <div id="followup">
 
-    <h2>Send a follow up message
-        <% if !incoming_message.safe_mail_from.nil? %>
-            to <%= incoming_message.safe_mail_from %>
-        <% end %>
-    </h2>
+    <% if incoming_message.nil? %>
+        <h2>Send a follow up message 
+        to '<%=h @info_request.public_body.name %>'
+        </h2>
+    <% else %>
+        <h2>Send a follow up message
+            <% if !incoming_message.safe_mail_from.nil? %>
+                to <%= incoming_message.safe_mail_from %>
+            <% end %>
+        </h2>
+    <% end %>
 
-    <p>Use this if the public body has asked for clarification of
-    your request.</p>
+    <p>Use this to tell the public body something, such as to clarify
+    your request, or if they are late responding.</p>
 
     <% form_for(:outgoing_message, @outgoing_message) do |o| %>
         <p>
diff --git a/app/views/request/show_response.rhtml b/app/views/request/show_response.rhtml
index 461639046..77d03214d 100644
--- a/app/views/request/show_response.rhtml
+++ b/app/views/request/show_response.rhtml
@@ -1,4 +1,6 @@
-<% if @incoming_message.recently_arrived %>
+<% if @incoming_message.nil? %>
+    <% @title = "Send follow up to '" + h(@info_request.title) + "'" %>
+<% elsif @incoming_message.recently_arrived %>
     <% @title = "New response to '" + h(@info_request.title) + "'" %>
 <% else %>
     <% @title = "Response to '" + h(@info_request.title) + "'" %>
@@ -8,20 +10,28 @@
 
 <div id="show_response_view">
     <% if @is_owning_user %>
-        <% if @incoming_message.recently_arrived %>
+        <% if @incoming_message.nil? %>
+            <h2>Your last message sent for request '<%= request_link @info_request %>'</h2>
+        <% elsif @incoming_message.recently_arrived %>
             <h2>New response to your request '<%= request_link @info_request %>'</h2>
         <% else %>
             <h2>Response to your request '<%= request_link @info_request %>'</h2>
         <% end %>
     <% else %>
-        <% if @incoming_message.recently_arrived %>
+        <% if @incoming_message.nil? %>
+            <h2>Last message sent for FOI request '<%= request_link @info_request %>'</h2>
+        <% elsif @incoming_message.recently_arrived %>
             <h2>New response to FOI request '<%= request_link @info_request %>'</h2>
         <% else %>
             <h2>Response to FOI request '<%= request_link @info_request %>'</h2>
         <% end %>
     <% end %>
 
-    <%= render :partial => 'correspondence', :locals => { :info_request_event => nil, :incoming_message => @incoming_message } %>
+    <% if @incoming_message.nil? %>
+        <%= render :partial => 'correspondence', :locals => { :info_request_event => @info_request.get_last_outgoing_event, :incoming_message => nil } %>
+    <% else %>
+        <%= render :partial => 'correspondence', :locals => { :info_request_event => nil, :incoming_message => @incoming_message } %>
+    <% end %>
 
     <div id="show_response_followup">
         <%= render :partial => 'followup', :locals => { :incoming_message => @incoming_message } %>
diff --git a/app/views/request_mailer/overdue_alert.rhtml b/app/views/request_mailer/overdue_alert.rhtml
new file mode 100644
index 000000000..2a0847220
--- /dev/null
+++ b/app/views/request_mailer/overdue_alert.rhtml
@@ -0,0 +1,13 @@
+<%= @info_request.public_body.name %> are late. 
+
+They have not replied to your FOI request '<%= @info_request.title %>' within
+the 20 working days they are allowed by law.
+
+Click on the link below to send a message to <%= @info_request.public_body.name
+%> reminding them to reply to your request.
+
+<%=@url%>
+
+-- the foi.mysociety.org team
+
+
diff --git a/config/crontab.ugly b/config/crontab.ugly
index 566fceb32..a580ce8ac 100644
--- a/config/crontab.ugly
+++ b/config/crontab.ugly
@@ -4,11 +4,14 @@
 # Copyright (c) 2008 UK Citizens Online Democracy. All rights reserved.
 # Email: francis@mysociety.org. WWW: http://www.mysociety.org/
 #
-# $Id: crontab.ugly,v 1.2 2008-02-13 12:10:41 francis Exp $
+# $Id: crontab.ugly,v 1.3 2008-02-22 01:58:37 francis Exp $
 
 PATH=/usr/local/bin:/usr/bin:/bin
 MAILTO=team@mysociety.org
 
+# Once an hour
+39 0 * * * !!(*= $user *)!! run-with-lockfile -n /data/vhost/!!(*= $vhost *)!!/alert-overdue-requests.lock /data/vhost/!!(*= $vhost *)!!/mysociety/foi/script/alert-overdue-requests || echo "stalled?"
+
 # 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?"
+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?"
 
diff --git a/config/routes.rb b/config/routes.rb
index 80055713c..3ecfdb373 100644
--- a/config/routes.rb
+++ b/config/routes.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: routes.rb,v 1.35 2008-02-21 18:32:43 francis Exp $
+# $Id: routes.rb,v 1.36 2008-02-22 01:58:37 francis Exp $
 
 ActionController::Routing::Routes.draw do |map|
     # The priority is based upon order of creation: first created -> highest priority.
@@ -22,6 +22,7 @@ ActionController::Routing::Routes.draw do |map|
       request.new_request_to_body    '/new/:public_body_id',         :action => 'new'
       request.show_request     '/request/:id', :action => 'show'
       request.describe_state   '/request/:id/describe', :action => 'describe_state'
+      request.show_response_no_followup    '/request/:id/response', :action => 'show_response'
       request.show_response    '/request/:id/response/:incoming_message_id', :action => 'show_response'
       request.get_attachment   '/request/:id/response/:incoming_message_id/attach/:part/*file_name', :action => 'get_attachment'
     end
diff --git a/db/migrate/035_track_overdue_alerts.rb b/db/migrate/035_track_overdue_alerts.rb
new file mode 100644
index 000000000..10e92b690
--- /dev/null
+++ b/db/migrate/035_track_overdue_alerts.rb
@@ -0,0 +1,19 @@
+class TrackOverdueAlerts < ActiveRecord::Migration
+    def self.up
+        create_table :user_info_request_sent_alerts do |t|
+            t.column :user_id, :integer
+            t.column :info_request_id, :integer
+
+            t.column :alert_type, :string
+        end
+
+        execute "ALTER TABLE user_info_request_sent_alerts ADD CONSTRAINT fk_info_request_sent_alerts_user FOREIGN KEY (user_id) REFERENCES users(id)"
+        execute "ALTER TABLE user_info_request_sent_alerts ADD CONSTRAINT fk_info_request_sent_alerts_info_request FOREIGN KEY (info_request_id) REFERENCES info_requests(id)"
+     end
+
+    def self.down
+        execute "ALTER TABLE user_info_request_sent_alerts DROP CONSTRAINT fk_info_request_sent_alerts_user"
+        execute "ALTER TABLE user_info_request_sent_alerts DROP CONSTRAINT fk_info_request_sent_alerts_info_request"
+        drop_table :user_info_request_sent_alerts
+    end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 4037e75cf..dfcc7e884 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -9,7 +9,7 @@
 #
 # It's strongly recommended to check this file into your version control system.
 
-ActiveRecord::Schema.define(:version => 34) do
+ActiveRecord::Schema.define(:version => 35) do
 
   create_table "incoming_messages", :force => true do |t|
     t.integer  "info_request_id",                    :null => false
@@ -101,6 +101,12 @@ ActiveRecord::Schema.define(:version => 34) do
   add_index "sessions", ["session_id"], :name => "index_sessions_on_session_id"
   add_index "sessions", ["updated_at"], :name => "index_sessions_on_updated_at"
 
+  create_table "user_info_request_sent_alerts", :force => true do |t|
+    t.integer "user_id"
+    t.integer "info_request_id"
+    t.string  "alert_type"
+  end
+
   create_table "users", :force => true do |t|
     t.string   "email",                              :null => false
     t.string   "name",                               :null => false
diff --git a/script/alert-overdue-requests b/script/alert-overdue-requests
new file mode 100755
index 000000000..e496c569a
--- /dev/null
+++ b/script/alert-overdue-requests
@@ -0,0 +1,7 @@
+#!/bin/bash
+
+LOC=`dirname $0`
+
+$LOC/runner 'RequestMailer.alert_overdue_requests()'
+
+
diff --git a/todo.txt b/todo.txt
index c6c5c279c..14bf15dbb 100644
--- a/todo.txt
+++ b/todo.txt
@@ -14,9 +14,9 @@ BAILII - relationship with law courts, robots.txt ?
 Status of messages stuff
 ========================
 
-When response is late, email the user
+Find transport/defence or rename them on front page
 
-Alert user if working days table not up to date, and offer followup
+Alert if working days table not up to date
 
 Adam's woes:
 http://foi.mysociety.org/request/18/response/31
@@ -31,8 +31,15 @@ Go through all requests and check status is shiny
 Next
 ====
 
+Late followup alerts are only sent first time message goes into that state :(
+
+Spacing on 
+    http://localhost:3000/request/113/response#show_response_followup
+is screwy
+
 Browse all public bodies from front page
 Make front page somehow a bit prettier
+Remove the juicier sidebar on request pages
 
 Sort the requests by when something last happened to them (this needs thought as to what
 sort orders we need)