Move TMail monkeypatches out of incoming_message into

8 files changed, 65 insertions, 57 deletions

diff --git a/app/models/incoming_message.rb b/app/models/incoming_message.rb
index 51c63ae65..4342a226c 100644
--- a/app/models/incoming_message.rb
+++ b/app/models/incoming_message.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: incoming_message.rb,v 1.169 2008-11-21 01:09:56 francis Exp $
+# $Id: incoming_message.rb,v 1.170 2008-11-21 01:50:06 francis Exp $
 
 # TODO
 # Move some of the (e.g. quoting) functions here into rblib, as they feel
@@ -30,54 +30,12 @@ require 'rexml/document'
 require 'zip/zip'
 require 'mahoro'
 
+# Monkeypatch! Adding some extra members to store extra info in.
 module TMail
     class Mail
         attr_accessor :url_part_number
         attr_accessor :rfc822_attachment # when a whole email message is attached as text
         attr_accessor :within_rfc822_attachment # for parts within a message attached as text (for getting subject mainly)
-
-        # Monkeypatch! (check to see if this becomes a standard function in
-        # TMail::Mail, then use that, whatever it is called)
-        def self.get_part_file_name(part)
-            file_name = (part['content-location'] &&
-                          part['content-location'].body) ||
-                        part.sub_header("content-type", "name") ||
-                        part.sub_header("content-disposition", "filename")
-        end
-
-        # Monkeypatch! :)
-        # Returns the name of the person a message is from, or nil if there isn't
-        # one or if there is only an email address.
-        def safe_from
-            if self.from and (not self.friendly_from.include?('@'))
-                return self.friendly_from
-            else 
-                return nil
-            end
-        end
-
-    end
-
-    class Address
-        # Monkeypatch!
-        def Address.encode_quoted_string(text)
-            if text.match(/[^A-Za-z0-9!#\$%&'*+\-\/=?^_`{|}~]/)
-                # Contains characters which aren't valid in atoms, so make a
-                # quoted-pair instead.
-                text.gsub!(/(["\\])/, "\\\\\\1")
-                text = '"' + text + '"'
-            end
-            return text
-        end
-
-        # Monkeypatch!
-        def quoted_full
-            if self.name
-                Address.encode_quoted_string(self.name) + " <" + self.spec + ">"
-            else
-                self.spec
-            end
-        end
     end
 end
 
diff --git a/app/models/info_request.rb b/app/models/info_request.rb
index 8e3b26b5e..7979c6f04 100644
--- a/app/models/info_request.rb
+++ b/app/models/info_request.rb
@@ -23,7 +23,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.155 2008-11-10 13:31:48 francis Exp $
+# $Id: info_request.rb,v 1.156 2008-11-21 01:50:06 francis Exp $
 
 require 'digest/sha1'
 require File.join(File.dirname(__FILE__),'../../vendor/plugins/acts_as_xapian/lib/acts_as_xapian')
@@ -142,7 +142,7 @@ public
         return self.magic_email("request-")
     end
     def incoming_name_and_email
-        return self.user.name + " <" + self.incoming_email + ">"
+        return TMail::Address.encode_quoted_string(self.user.name) + " <" + self.incoming_email + ">"
     end
 
     # Subject lines for emails about the request
@@ -510,7 +510,7 @@ public
         return self.public_body.request_email
     end
     def recipient_name_and_email
-        return self.law_used_short + " requests at " + self.public_body.short_or_long_name + " <" + self.recipient_email + ">"
+        return TMail::Address.encode_quoted_string(self.law_used_short + " requests at " + self.public_body.short_or_long_name) + " <" + self.recipient_email + ">"
     end
 
     # History of some things that have happened
diff --git a/app/models/request_mailer.rb b/app/models/request_mailer.rb
index 15d3043a0..bf9327546 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.66 2008-11-10 20:05:28 francis Exp $
+# $Id: request_mailer.rb,v 1.67 2008-11-21 01:50:06 francis Exp $
 
 class RequestMailer < ApplicationMailer
     
@@ -35,7 +35,6 @@ class RequestMailer < ApplicationMailer
         if incoming_message_followup.nil? || !incoming_message_followup.valid_to_reply_to?
             return info_request.recipient_name_and_email
         else
-            IncomingMessage # get Monkeypatch!
             return incoming_message_followup.mail.from_addrs[0].quoted_full
         end
     end
@@ -90,7 +89,6 @@ class RequestMailer < ApplicationMailer
     # Incoming message arrived for a request, but new responses have been stopped.
     def stopped_responses(info_request, email)
         @from = contact_from_name_and_email
-        IncomingMessage # get Monkeypatch!
         @recipients = email.from_addrs[0].quoted_full
         @subject = "Your response to an FOI request was not delivered"
         attachment :content_type => 'message/rfc822', :body => email.body
diff --git a/app/models/user.rb b/app/models/user.rb
index 162d86d35..d4fbceb21 100644
--- a/app/models/user.rb
+++ b/app/models/user.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: user.rb,v 1.74 2008-10-28 13:04:20 francis Exp $
+# $Id: user.rb,v 1.75 2008-11-21 01:50:06 francis Exp $
 
 require 'digest/sha1'
 
@@ -154,7 +154,7 @@ class User < ActiveRecord::Base
 
     # For use in to/from in email messages
     def name_and_email
-        return self.name + " <" + self.email + ">"
+        return TMail::Address.encode_quoted_string(self.name) + " <" + self.email + ">"
     end
 
     # The "internal admin" is a special user for internal use.
diff --git a/config/environment.rb b/config/environment.rb
index 6b23b6e52..b50cfb6dd 100644
--- a/config/environment.rb
+++ b/config/environment.rb
@@ -120,6 +120,56 @@ module WillPaginate
     end
 end
 
+# These mainly used in app/models/incoming_message.rb
+module TMail
+    class Mail
+        # Monkeypatch! (check to see if this becomes a standard function in
+        # TMail::Mail, then use that, whatever it is called)
+        def self.get_part_file_name(part)
+            file_name = (part['content-location'] &&
+                          part['content-location'].body) ||
+                        part.sub_header("content-type", "name") ||
+                        part.sub_header("content-disposition", "filename")
+        end
+
+        # Monkeypatch! :)
+        # Returns the name of the person a message is from, or nil if there isn't
+        # one or if there is only an email address.
+        def safe_from
+            if self.from and (not self.friendly_from.include?('@'))
+                return self.friendly_from
+            else 
+                return nil
+            end
+        end
+
+    end
+
+    class Address
+        # Monkeypatch!
+        def Address.encode_quoted_string(text)
+            # XXX have added space to this, so we don't excessive quoting
+            if text.match(/[^A-Za-z0-9!#\$%&'*+\-\/=?^_`{|}~ ]/)
+                # Contains characters which aren't valid in atoms, so make a
+                # quoted-pair instead.
+                text.gsub!(/(["\\])/, "\\\\\\1")
+                text = '"' + text + '"'
+            end
+            return text
+        end
+
+        # Monkeypatch!
+        def quoted_full
+            if self.name
+                Address.encode_quoted_string(self.name) + " <" + self.spec + ">"
+            else
+                self.spec
+            end
+        end
+    end
+end
+
+
 # XXX temp debug for SQL logging production sites
 #ActiveRecord::Base.logger = Logger.new(STDOUT)
 
diff --git a/spec/fixtures/raw_emails.yml b/spec/fixtures/raw_emails.yml
index 6e43b1a9a..46f7ec09a 100644
--- a/spec/fixtures/raw_emails.yml
+++ b/spec/fixtures/raw_emails.yml
@@ -1,8 +1,8 @@
 useless_raw_email: 
   id: 1
   data: |
-    From: FOI Person <foiperson@localhost>
-    To: Bob Smith <bob@localhost>
+    From: "FOI Person" <foiperson@localhost>
+    To: "Bob Smith" <bob@localhost>
     Date: Tue, 13 Nov 2007 11:39:55 +0000
     NoCc: foi+request-1-4b571715@cat
     Bcc: 
diff --git a/spec/models/incoming_message_spec.rb b/spec/models/incoming_message_spec.rb
index 63a1680c4..c1c846275 100644
--- a/spec/models/incoming_message_spec.rb
+++ b/spec/models/incoming_message_spec.rb
@@ -11,6 +11,11 @@ describe IncomingMessage, " when dealing with incoming mail" do
         @im.sent_at.should == @im.mail.date
     end
 
+    it "should be able to parse emails with quoted commas in" do
+        em = "\"Clare College, Cambridge\" <test@test.test>"
+        TMail::Address.parse(em)
+    end
+
 end
 
 
diff --git a/todo.txt b/todo.txt
index 8a6f43a6c..78c47228c 100644
--- a/todo.txt
+++ b/todo.txt
@@ -229,9 +229,6 @@ updating
 (10:32:52) Matthew: their function is called autogrow_textarea() by the way, if you just want to look at it...
  thanks :) I won't do it now as there are more important things, I was just accidentally impressed
 
-Properly escape all name_and_email functions
-    call our new TMail.Address.encode_quoted_string monkeypatch function
-
 Including 'Request for information' in the subject line of your e-mail will assist us in prioritising your request.
 http://www.parliament.uk/parliamentary_publications_and_archives/freedom_of_information_in_the_house_of_lords/lords__foi___how_to_obtain_information.cfm
 XXX do we need specifically that text, is it better than "Freedom of Information request" which we put now?