Don't send overdue alerts, or request for clarification alerts to banned users

4 files changed, 49 insertions, 5 deletions

diff --git a/app/models/request_mailer.rb b/app/models/request_mailer.rb
index c5f5ab2b5..ad227c8d9 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.86 2009-10-03 02:22:18 francis Exp $
+# $Id: request_mailer.rb,v 1.87 2009-10-03 09:47:16 francis Exp $
 
 class RequestMailer < ApplicationMailer
     
@@ -270,14 +270,18 @@ class RequestMailer < ApplicationMailer
                 # 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 = ? 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
+                    # Alert not yet sent for this user, so send it
                     #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)
+                    # Only send the alert if the user can act on it by making a followup
+                    # (otherwise they are banned, and there is no point sending it)
+                    if info_request.user.can_make_followup?
+                        RequestMailer.deliver_overdue_alert(info_request, info_request.user)
+                    end
                     store_sent.save!
                     #STDERR.puts "sent " + info_request.user.email
                 end
@@ -343,7 +347,11 @@ class RequestMailer < ApplicationMailer
                 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)
+                # Only send the alert if the user can act on it by making a followup
+                # (otherwise they are banned, and there is no point sending it)
+                if info_request.user.can_make_followup?
+                    RequestMailer.deliver_not_clarified_alert(info_request, last_response_message)
+                end
                 store_sent.save!
                 #STDERR.puts "sent " + info_request.user.email
             end
diff --git a/spec/controllers/request_controller_spec.rb b/spec/controllers/request_controller_spec.rb
index 4f8e5c2d6..e8c4e8ba0 100644
--- a/spec/controllers/request_controller_spec.rb
+++ b/spec/controllers/request_controller_spec.rb
@@ -832,6 +832,17 @@ describe RequestController, "sending overdue request alerts" do
         assigns[:info_request].should == info_requests(:naughty_chicken_request)
     end
 
+    it "should send not actualy send the overdue alert if the user is banned" do
+        user = info_requests(:naughty_chicken_request).user
+        user.ban_text = 'Banned'
+        user.save!
+
+        RequestMailer.alert_overdue_requests
+
+        deliveries = ActionMailer::Base.deliveries
+        deliveries.size.should == 0
+    end
+
 end
 
 describe RequestController, "sending unclassified new response reminder alerts" do
@@ -842,7 +853,7 @@ describe RequestController, "sending unclassified new response reminder alerts"
         RequestMailer.alert_new_response_reminders
 
         deliveries = ActionMailer::Base.deliveries
-        deliveries.size.should == 2 # sufficiently late it sends reminder too
+        deliveries.size.should == 3 # sufficiently late it sends reminders too
         mail = deliveries[0]
         mail.body.should =~ /To let us know/
         mail.to_addrs.to_s.should == info_requests(:fancy_dog_request).user.name_and_email
@@ -892,6 +903,24 @@ describe RequestController, "clarification required alerts" do
         assigns[:info_request].should == info_requests(:fancy_dog_request)
     end
 
+    it "should not send an alert if you are banned" do
+        ir = info_requests(:fancy_dog_request)
+        ir.set_described_state('waiting_clarification')
+
+        ir.user.ban_text = 'Banned'
+        ir.user.save!
+
+        # 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 = '" + (Time.now - 5.days).strftime("%Y-%m-%d %H:%M:%S") + "' where id = " + ir.id.to_s
+        ir.reload
+
+        RequestMailer.alert_not_clarified_request
+
+        deliveries = ActionMailer::Base.deliveries
+        deliveries.size.should == 0
+    end
+
 end
 
 describe RequestController, "comment alerts" do
diff --git a/spec/models/track_mailer_spec.rb b/spec/models/track_mailer_spec.rb
index 4cf85c31f..0b1db6003 100644
--- a/spec/models/track_mailer_spec.rb
+++ b/spec/models/track_mailer_spec.rb
@@ -118,6 +118,7 @@ describe TrackMailer do
             mail = deliveries[0]
 
             mail['Auto-Submitted'].to_s.should == 'auto-generated'
+            mail['Precedence'].to_s.should == 'bulk'
         end
     end
 
diff --git a/todo.txt b/todo.txt
index cfcb4caf8..8be54cb8d 100644
--- a/todo.txt
+++ b/todo.txt
@@ -59,6 +59,7 @@ julian.todd@section44.whatdotheyknow.com
 * Use FOI code allocated by authority to work out where emails are to go
 * Second request to same authority by same person - tell them to be sure
 to use the right email
+* Improve routing from Exim so copes with addresses not having request- prefix.
 
 check-recent-requests-sent probably doesn't work, as exim log lines wouldn't
 be load in case where the envelope from gets broken?
@@ -79,6 +80,9 @@ and resend messages to them
 Perhaps fold up request pages more by default - don't show known acknowledgements until
 you click and some (javascript) expands them.
 
+Some people want the fold/unfold of quoted sections to be javascripty as well.
+Esp. when filling in a form on the same page.
+
 Somehow make clear that a "rejection because referring to info already
 in public domain" should really be marked sucessful.
 
@@ -404,6 +408,8 @@ Do conversion tracking on endpoints in WDTK, advertise perhaps TWFY, or perhaps
 Advertise WDTK search queries on TWFY
 Advertise alerts on end pages with WDTK
 
+Forms to search this user, this request, and this authority on their pages
+
 Search FAQ and other help pages with normal search
 
 Make text boxes autogrow as you type into them.