If request is deleted, then send email on to admin, rather than raising an

exception that goes to the public body in a delivery failure.

3 files changed, 33 insertions, 16 deletions

diff --git a/app/models/info_request.rb b/app/models/info_request.rb
index 0f3fa4e39..fb8c513e7 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.67 2008-03-16 23:32:11 francis Exp $
+# $Id: info_request.rb,v 1.68 2008-03-17 18:53:30 francis Exp $
 
 require 'digest/sha1'
 
@@ -76,7 +76,7 @@ class InfoRequest < ActiveRecord::Base
 
     $do_solr_index = false
     $do_solr_index_marking = false
-    def self.update_solr_index
+    def InfoRequest.update_solr_index
         #STDERR.puts "self.update_solr_index"
         $do_solr_index = true
 
@@ -184,7 +184,7 @@ public
     # only they are sent the email address with the has in it. (We don't check
     # the prefix and domain, as sometimes those change, or might be elided by
     # copying an email, and that doesn't matter)
-    def self.find_by_incoming_email(incoming_email)
+    def InfoRequest.find_by_incoming_email(incoming_email)
         incoming_email =~ /request-(\d+)-([a-z0-9]+)/
         id = $1.to_i
         hash = $2
@@ -192,7 +192,7 @@ public
         return self.find_by_magic_email(id, hash)
     end
 
-    def self.find_by_envelope_email(incoming_email)
+    def InfoRequest.find_by_envelope_email(incoming_email)
         incoming_email =~ /request-bounce-(\d+)-([a-z0-9]+)/
         id = $1.to_i
         hash = $2
@@ -205,7 +205,7 @@ public
     # repeated requests, say once a quarter for time information, then might need to do that.
     # XXX this *should* also check outgoing message joined to is an initial
     # request (rather than follow up)
-    def self.find_by_existing_request(title, public_body_id, body)
+    def InfoRequest.find_by_existing_request(title, public_body_id, body)
         return InfoRequest.find(:first, :conditions => [ 'title = ? and public_body_id = ? and outgoing_messages.body = ?', title, public_body_id, body ], :include => [ :outgoing_messages ] )
     end
 
@@ -465,26 +465,34 @@ public
     end
 
 
-    protected
-
     # Called by incoming_email and envelope_email
     def magic_email(prefix_part)
         raise "id required to make magic" if not self.id
+        return InfoRequest.magic_email_for_id(prefix_part, self.id)
+    end
+
+    def InfoRequest.magic_email_for_id(prefix_part, id) 
         magic_email = MySociety::Config.get("INCOMING_EMAIL_PREFIX", "") 
-        magic_email += prefix_part + self.id.to_s 
-        magic_email += "-" + Digest::SHA1.hexdigest(self.id.to_s + MySociety::Config.get("INCOMING_EMAIL_SECRET", 'dummysecret'))[0,8]
+        magic_email += prefix_part + id.to_s
+        magic_email += "-" + Digest::SHA1.hexdigest(id.to_s + MySociety::Config.get("INCOMING_EMAIL_SECRET", 'dummysecret'))[0,8]
         magic_email += "@" + MySociety::Config.get("INCOMING_EMAIL_DOMAIN", "localhost")
         return magic_email
     end
 
     # Called by find_by_incoming_email and find_by_envelope_email
-    def self.find_by_magic_email(id, hash)
+    def InfoRequest.find_by_magic_email(id, hash)
         expected_hash = Digest::SHA1.hexdigest(id.to_s + MySociety::Config.get("INCOMING_EMAIL_SECRET", 'dummysecret'))[0,8]
         #print "expected: " + expected_hash + "\nhash: " + hash + "\n"
         if hash != expected_hash
             return nil
         else
-            return self.find(id)
+            begin
+                return self.find(id)
+            rescue ActiveRecord::RecordNotFound
+                # so error email is sent to admin, rather than the exception sending weird
+                # error to the public body.
+                return nil
+            end
         end
     end
 
diff --git a/spec/models/info_request_spec.rb b/spec/models/info_request_spec.rb
index add67b44f..1607c85ba 100644
--- a/spec/models/info_request_spec.rb
+++ b/spec/models/info_request_spec.rb
@@ -17,6 +17,12 @@ describe InfoRequest, " when emailing" do
         found_info_request.should == (@info_request)
     end
 
+    it "should return nil when receiving email for a deleted request" do
+        deleted_request_address = InfoRequest.magic_email_for_id("request-", 98765)  
+        found_info_request = InfoRequest.find_by_incoming_email(deleted_request_address)
+        found_info_request.should be_nil
+    end
+
 end
 
 
diff --git a/todo.txt b/todo.txt
index 9a7156320..e0201e556 100644
--- a/todo.txt
+++ b/todo.txt
@@ -1,8 +1,3 @@
-Add list of things to do
-
-
-
-
 Search:
 
 Gah - test to see if it queries solr when new message arrives
@@ -16,6 +11,9 @@ Search by tags on public bodies
 
 Check cron jobs are running
 
+
+
+
 FOI requests to use to test it
 ==============================
 
@@ -31,6 +29,10 @@ BAILII - relationship with law courts, robots.txt ?
 Next
 ====
 
+Make clear about DPA vs. FOI
+Hidden flag
+
+
 Page of new responses - so I don't have to go into email
 
 Need something to mark contact as bad, e.g. for university of huddersfield
@@ -185,6 +187,7 @@ Add geographical location to council import
 Quoting fixing TODO:
     http://www.whatdotheyknow.com/request/64/response/102
     http://www.whatdotheyknow.com/request/55/response/96
+    http://www.whatdotheyknow.com/request/71/response/108
 
 Sources of public bodies
 ========================