Get rid of special envelope from -bounce addresses, as they are no good since

enough public authorities mail them with non-automatic messages.

18 files changed, 62 insertions, 78 deletions

diff --git a/app/models/contact_mailer.rb b/app/models/contact_mailer.rb
index e3d1d3c71..b36d5f165 100644
--- a/app/models/contact_mailer.rb
+++ b/app/models/contact_mailer.rb
@@ -4,7 +4,7 @@
 # Copyright (c) 2008 UK Citizens Online Democracy. All rights reserved.
 # Email: francis@mysociety.org; WWW: http://www.mysociety.org/
 #
-# $Id: contact_mailer.rb,v 1.5 2008-03-13 12:33:40 francis Exp $
+# $Id: contact_mailer.rb,v 1.6 2008-03-21 14:04:29 francis Exp $
 
 class ContactMailer < ApplicationMailer
 
@@ -18,6 +18,12 @@ class ContactMailer < ApplicationMailer
         }
     end
 
+    # We always set Reply-To when we set Sender to be different from From,
+    # since some email clients seem to erroneously use the envelope from when
+    # they shouldn't, and this might help. (Have had mysterious cases of a
+    # reply coming in duplicate from a public body to both From and envelope
+    # from)
+ 
     # Send message to another user
     def user_message(from_user, recipient_user, from_user_url, subject, message)
         @from = from_user.name_and_email
diff --git a/app/models/contact_validator.rb b/app/models/contact_validator.rb
index ce306ddbd..d4f1d0a7b 100644
--- a/app/models/contact_validator.rb
+++ b/app/models/contact_validator.rb
@@ -1,5 +1,5 @@
 # == Schema Information
-# Schema version: 43
+# Schema version: 44
 #
 # Table name: contact_validators
 #
@@ -15,7 +15,7 @@
 # Copyright (c) 2008 UK Citizens Online Democracy. All rights reserved.
 # Email: francis@mysociety.org; WWW: http://www.mysociety.org/
 #
-# $Id: contact_validator.rb,v 1.6 2008-03-18 19:18:51 francis Exp $
+# $Id: contact_validator.rb,v 1.7 2008-03-21 14:04:29 francis Exp $
 
 class ContactValidator < ActiveRecord::BaseWithoutTable
     column :name, :string
diff --git a/app/models/incoming_message.rb b/app/models/incoming_message.rb
index 306c9b9e8..4b56f0cc4 100644
--- a/app/models/incoming_message.rb
+++ b/app/models/incoming_message.rb
@@ -1,5 +1,5 @@
 # == Schema Information
-# Schema version: 43
+# Schema version: 44
 #
 # Table name: incoming_messages
 #
@@ -8,7 +8,6 @@
 #  raw_data        :text            not null
 #  created_at      :datetime        not null
 #  updated_at      :datetime        not null
-#  is_bounce       :boolean         default(false), not null
 #
 
 # models/incoming_message.rb:
@@ -18,7 +17,7 @@
 # Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved.
 # Email: francis@mysociety.org; WWW: http://www.mysociety.org/
 #
-# $Id: incoming_message.rb,v 1.67 2008-03-18 19:18:51 francis Exp $
+# $Id: incoming_message.rb,v 1.68 2008-03-21 14:04:29 francis Exp $
 
 
 # TODO
@@ -140,7 +139,6 @@ class IncomingMessage < ActiveRecord::Base
             text = text.gsub(self.info_request.public_body.request_email, "[" + self.info_request.public_body.short_or_long_name + " request email]")
         end
         text = text.gsub(self.info_request.incoming_email, "[FOI #" + self.info_request.id.to_s + " email]")
-        text = text.gsub(self.info_request.envelope_email, "[FOI #" + self.info_request.id.to_s + " bounce email]")
         text = text.gsub(MySociety::Config.get("CONTACT_EMAIL", 'contact@localhost'), "[WhatDoTheyKnow contact email]")
         return text
     end
diff --git a/app/models/info_request.rb b/app/models/info_request.rb
index a6ec8d80b..647d884f8 100644
--- a/app/models/info_request.rb
+++ b/app/models/info_request.rb
@@ -1,5 +1,5 @@
 # == Schema Information
-# Schema version: 43
+# Schema version: 44
 #
 # Table name: info_requests
 #
@@ -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.69 2008-03-18 19:18:51 francis Exp $
+# $Id: info_request.rb,v 1.70 2008-03-21 14:04:29 francis Exp $
 
 require 'digest/sha1'
 
@@ -162,15 +162,6 @@ public
         return self.user.name + " <" + self.incoming_email + ">"
     end
 
-    # Modified version of incoming_email to use in the envelope from, for
-    # bounce messages.
-    def envelope_email
-        return self.magic_email("request-bounce-")
-    end
-    def envelope_name_and_email
-        return self.user.name + " <" + self.envelope_email + ">"
-    end
-
     # Subject lines for emails about the request
     def email_subject_request
         'Freedom of Information request - ' + self.title
@@ -185,15 +176,10 @@ public
     # the prefix and domain, as sometimes those change, or might be elided by
     # copying an email, and that doesn't matter)
     def InfoRequest.find_by_incoming_email(incoming_email)
-        incoming_email =~ /request-(\d+)-([a-z0-9]+)/
-        id = $1.to_i
-        hash = $2
-
-        return self.find_by_magic_email(id, hash)
-    end
-
-    def InfoRequest.find_by_envelope_email(incoming_email)
-        incoming_email =~ /request-bounce-(\d+)-([a-z0-9]+)/
+        # The optional bounce- dates from when we used to have separate emails for the envelope from.
+        # (that was abandoned because councils would send hand written responses to them, not just
+        # bounce messages)
+        incoming_email =~ /request-(?:bounce-)?(\d+)-([a-z0-9]+)/
         id = $1.to_i
         hash = $2
 
@@ -210,12 +196,11 @@ public
     end
 
     # A new incoming email to this request
-    def receive(email, raw_email, is_bounce)
+    def receive(email, raw_email)
         incoming_message = IncomingMessage.new
 
         ActiveRecord::Base.transaction do
             incoming_message.raw_data = raw_email
-            incoming_message.is_bounce = is_bounce
             incoming_message.info_request = self
             incoming_message.save!
 
@@ -464,8 +449,8 @@ public
         end
     end
 
-
-    # Called by incoming_email and envelope_email
+    # Called by incoming_email - and used to be called to generate separate
+    # envelope from address until we abandoned it.
     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)
@@ -479,7 +464,8 @@ public
         return magic_email
     end
 
-    # Called by find_by_incoming_email and find_by_envelope_email
+    # Called by find_by_incoming_email - and used to be called by separate
+    # function for envelope from address, until we abandoned it.
     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"
diff --git a/app/models/info_request_event.rb b/app/models/info_request_event.rb
index 7093c808d..41408787e 100644
--- a/app/models/info_request_event.rb
+++ b/app/models/info_request_event.rb
@@ -1,5 +1,5 @@
 # == Schema Information
-# Schema version: 43
+# Schema version: 44
 #
 # Table name: info_request_events
 #
@@ -16,7 +16,7 @@
 # Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved.
 # Email: francis@mysociety.org; WWW: http://www.mysociety.org/
 #
-# $Id: info_request_event.rb,v 1.22 2008-03-18 19:18:51 francis Exp $
+# $Id: info_request_event.rb,v 1.23 2008-03-21 14:04:29 francis Exp $
 
 class InfoRequestEvent < ActiveRecord::Base
     belongs_to :info_request
diff --git a/app/models/outgoing_message.rb b/app/models/outgoing_message.rb
index f802de5d7..6ba085fe0 100644
--- a/app/models/outgoing_message.rb
+++ b/app/models/outgoing_message.rb
@@ -1,5 +1,5 @@
 # == Schema Information
-# Schema version: 43
+# Schema version: 44
 #
 # Table name: outgoing_messages
 #
@@ -21,7 +21,7 @@
 # Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved.
 # Email: francis@mysociety.org; WWW: http://www.mysociety.org/
 #
-# $Id: outgoing_message.rb,v 1.36 2008-03-18 19:18:51 francis Exp $
+# $Id: outgoing_message.rb,v 1.37 2008-03-21 14:04:29 francis Exp $
 
 class OutgoingMessage < ActiveRecord::Base
     belongs_to :info_request
diff --git a/app/models/post_redirect.rb b/app/models/post_redirect.rb
index a55e32ce1..e62d4b2a0 100644
--- a/app/models/post_redirect.rb
+++ b/app/models/post_redirect.rb
@@ -1,5 +1,5 @@
 # == Schema Information
-# Schema version: 43
+# Schema version: 44
 #
 # Table name: post_redirects
 #
@@ -25,7 +25,7 @@
 # Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved.
 # Email: francis@mysociety.org; WWW: http://www.mysociety.org/
 #
-# $Id: post_redirect.rb,v 1.24 2008-03-20 11:58:21 francis Exp $
+# $Id: post_redirect.rb,v 1.25 2008-03-21 14:04:29 francis Exp $
 
 require 'openssl' # for random bytes function
 
diff --git a/app/models/public_body.rb b/app/models/public_body.rb
index 10e66c1b6..3772e39c6 100644
--- a/app/models/public_body.rb
+++ b/app/models/public_body.rb
@@ -1,5 +1,5 @@
 # == Schema Information
-# Schema version: 43
+# Schema version: 44
 #
 # Table name: public_bodies
 #
@@ -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.40 2008-03-18 19:18:51 francis Exp $
+# $Id: public_body.rb,v 1.41 2008-03-21 14:04:29 francis Exp $
 
 require 'csv'
 require 'set'
diff --git a/app/models/public_body_tag.rb b/app/models/public_body_tag.rb
index 8fe1f86fa..a1d0cc9d0 100644
--- a/app/models/public_body_tag.rb
+++ b/app/models/public_body_tag.rb
@@ -1,5 +1,5 @@
 # == Schema Information
-# Schema version: 43
+# Schema version: 44
 #
 # Table name: public_body_tags
 #
@@ -15,7 +15,7 @@
 # Copyright (c) 2008 UK Citizens Online Democracy. All rights reserved.
 # Email: francis@mysociety.org; WWW: http://www.mysociety.org/
 #
-# $Id: public_body_tag.rb,v 1.4 2008-03-18 19:18:51 francis Exp $
+# $Id: public_body_tag.rb,v 1.5 2008-03-21 14:04:29 francis Exp $
 
 class PublicBodyTag < ActiveRecord::Base
     validates_presence_of :public_body
diff --git a/app/models/request_mailer.rb b/app/models/request_mailer.rb
index aac250095..8300422bb 100644
--- a/app/models/request_mailer.rb
+++ b/app/models/request_mailer.rb
@@ -4,21 +4,13 @@
 # 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.26 2008-03-19 05:26:31 francis Exp $
+# $Id: request_mailer.rb,v 1.27 2008-03-21 14:04:29 francis Exp $
 
 class RequestMailer < ApplicationMailer
     
-    # We always set Reply-To when we set Sender to be different from From,
-    # since some email clients seem to erroneously use the envelope from when
-    # they shouldn't, and this might help. (Have had mysterious cases of a
-    # reply coming in duplicate from a public body to both From and envelope
-    # from)
-    
     # Email to public body requesting info
     def initial_request(info_request, outgoing_message)
         @from = info_request.incoming_name_and_email
-        headers 'Sender' => info_request.envelope_name_and_email, 
-                'Reply-To' => @from
         @recipients = info_request.recipient_name_and_email
         @subject    = info_request.email_subject_request
         @body       = {:info_request => info_request, :outgoing_message => outgoing_message,
@@ -28,8 +20,6 @@ class RequestMailer < ApplicationMailer
     # Later message to public body regarding existing request
     def followup(info_request, outgoing_message, incoming_message_followup)
         @from = info_request.incoming_name_and_email
-        headers 'Sender' => info_request.envelope_name_and_email,
-                'Reply-To' => @from
         if incoming_message_followup.nil?
             @recipients = info_request.recipient_name_and_email
         else
@@ -113,25 +103,19 @@ class RequestMailer < ApplicationMailer
     def receive(email, raw_email)
         # Find which info requests the email is for
         reply_info_requests = []
-        bounce_info_requests = []
         for address in (email.to || []) + (email.cc || [])
             reply_info_request = InfoRequest.find_by_incoming_email(address)
             reply_info_requests.push(reply_info_request) if reply_info_request
-            bounce_info_request = InfoRequest.find_by_envelope_email(address)
-            bounce_info_requests.push(bounce_info_request) if bounce_info_request
         end
 
         # Nothing found
-        if reply_info_requests.size == 0 && bounce_info_requests.size == 0
+        if reply_info_requests.size == 0 
             RequestMailer.deliver_bounced_message(email)
         end
 
         # Send the message to each request, to be archived with it
         for reply_info_request in reply_info_requests
-            reply_info_request.receive(email, raw_email, false)
-        end
-        for bounce_info_request in bounce_info_requests
-            bounce_info_request.receive(email, raw_email, true)
+            reply_info_request.receive(email, raw_email)
         end
     end
 
diff --git a/app/models/user.rb b/app/models/user.rb
index 31d33a3ce..4c5e13f06 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -1,5 +1,5 @@
 # == Schema Information
-# Schema version: 43
+# Schema version: 44
 #
 # Table name: users
 #
@@ -20,7 +20,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.38 2008-03-18 19:18:51 francis Exp $
+# $Id: user.rb,v 1.39 2008-03-21 14:04:29 francis Exp $
 
 require 'digest/sha1'
 
diff --git a/app/models/user_info_request_sent_alert.rb b/app/models/user_info_request_sent_alert.rb
index cddb14b3a..99dea8f42 100644
--- a/app/models/user_info_request_sent_alert.rb
+++ b/app/models/user_info_request_sent_alert.rb
@@ -1,5 +1,5 @@
 # == Schema Information
-# Schema version: 43
+# Schema version: 44
 #
 # Table name: user_info_request_sent_alerts
 #
@@ -16,7 +16,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.5 2008-03-18 19:18:51 francis Exp $
+# $Id: user_info_request_sent_alert.rb,v 1.6 2008-03-21 14:04:29 francis Exp $
 
 class UserInfoRequestSentAlert < ActiveRecord::Base
     belongs_to :user
diff --git a/app/views/admin_request/show.rhtml b/app/views/admin_request/show.rhtml
index 2556513e6..f9fb4ce45 100644
--- a/app/views/admin_request/show.rhtml
+++ b/app/views/admin_request/show.rhtml
@@ -10,7 +10,6 @@
 <strong>Created by:</strong> <%= user_admin_link(@info_request.user) %> <br>
 <strong>Public body:</strong> <%= public_body_admin_link(@info_request.public_body) %> <br>
 <strong>Incoming email address:</strong> <%=h @info_request.incoming_email %> <br>
-<strong>Envelope email address:</strong> <%=h @info_request.envelope_email %> <br>
 </p>
 
 <p>
diff --git a/app/views/request/_correspondence.rhtml b/app/views/request/_correspondence.rhtml
index 3bb823fa1..8f91631f2 100644
--- a/app/views/request/_correspondence.rhtml
+++ b/app/views/request/_correspondence.rhtml
@@ -18,12 +18,7 @@ if not incoming_message.nil?
             <% if !incoming_message.safe_mail_from.nil? %>
                 <%= incoming_message.safe_mail_from %> of
             <% end %>
-            <%= public_body_link(@info_request.public_body) %>
-            <% if incoming_message.is_bounce %>
-                replied automatically
-            <% else %>
-                replied 
-            <% end %>
+            <%= public_body_link(@info_request.public_body) %> replied 
             (<%= link_to "link to this", show_response_url(:id => incoming_message.info_request.id, :incoming_message_id => incoming_message.id) %>,
             <%= link_to "reply", show_response_url(:id => incoming_message.info_request.id, :incoming_message_id => incoming_message.id) + "#show_response_followup" %>)
         </p>
diff --git a/db/migrate/044_remove_is_bounce.rb b/db/migrate/044_remove_is_bounce.rb
new file mode 100644
index 000000000..8e2307060
--- /dev/null
+++ b/db/migrate/044_remove_is_bounce.rb
@@ -0,0 +1,9 @@
+class RemoveIsBounce < ActiveRecord::Migration
+    def self.up
+        remove_column :incoming_messages, :is_bounce
+    end
+
+    def self.down
+        add_column :incoming_messages, :is_bounce, :boolean, :default => false
+    end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 5bcb2470e..2f304ddc7 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -9,14 +9,13 @@
 #
 # It's strongly recommended to check this file into your version control system.
 
-ActiveRecord::Schema.define(:version => 43) do
+ActiveRecord::Schema.define(:version => 44) do
 
   create_table "incoming_messages", :force => true do |t|
-    t.integer  "info_request_id",                    :null => false
-    t.text     "raw_data",                           :null => false
-    t.datetime "created_at",                         :null => false
-    t.datetime "updated_at",                         :null => false
-    t.boolean  "is_bounce",       :default => false, :null => false
+    t.integer  "info_request_id", :null => false
+    t.text     "raw_data",        :null => false
+    t.datetime "created_at",      :null => false
+    t.datetime "updated_at",      :null => false
   end
 
   create_table "info_request_events", :force => true do |t|
diff --git a/spec/models/info_request_spec.rb b/spec/models/info_request_spec.rb
index 1607c85ba..4e9230de7 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 recognise old style request-bounce- addresses" do
+        incoming_email = @info_request.magic_email("request-bounce-")
+        found_info_request = InfoRequest.find_by_incoming_email(incoming_email)
+        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)
diff --git a/todo.txt b/todo.txt
index c676c984e..ab0060465 100644
--- a/todo.txt
+++ b/todo.txt
@@ -1,3 +1,5 @@
+remove is_bounce
+
 Search:
 
 Gah - test to see if it queries solr when new message arrives